Skip to content

[SPARK-37860][UI] Fix taskindex in the stage page task event timeline #35160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jackylee-ch
Copy link
Contributor

What changes were proposed in this pull request?

This reverts commit 450b415.

Why are the changes needed?

In #32888, @shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use index.attempt or taskId to distinguish tasks within a stage, not taskId.attempt.
Thus #32888 was a wrong fix issue, we should revert it.

Does this PR introduce any user-facing change?

no

How was this patch tested?

origin test suites

@jackylee-ch
Copy link
Contributor Author

@HyukjinKwon
Copy link
Member

cc @gengliangwang too

@dongjoon-hyun
Copy link
Member

Thank you, @stczwd !

@sarutak
Copy link
Member

sarutak commented Jan 11, 2022

@stczwd Thank you for pointing out the issue! It LGTM and I'll merge this to master, branch-3.2, branch-3.1 and branch-3.0.

@sarutak sarutak closed this in 3d2fde5 Jan 11, 2022
sarutak pushed a commit that referenced this pull request Jan 11, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In #32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus #32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes #35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
sarutak pushed a commit that referenced this pull request Jan 11, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In #32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus #32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes #35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
sarutak pushed a commit that referenced this pull request Jan 11, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In #32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus #32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes #35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
@jackylee-ch jackylee-ch deleted the SPARK-37860 branch January 11, 2022 06:28
@jackylee-ch
Copy link
Contributor Author

Thanks @dongjoon-hyun @HyukjinKwon @sarutak

fishcus pushed a commit to fishcus/spark that referenced this pull request Jan 12, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In apache#32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus apache#32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes apache#35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
dchvn pushed a commit to dchvn/spark that referenced this pull request Jan 19, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In apache#32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus apache#32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes apache#35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
catalinii pushed a commit to lyft/spark that referenced this pull request Feb 22, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In apache#32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus apache#32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes apache#35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
catalinii pushed a commit to lyft/spark that referenced this pull request Mar 4, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In apache#32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus apache#32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes apache#35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
kazuyukitanimura pushed a commit to kazuyukitanimura/spark that referenced this pull request Aug 10, 2022
### What changes were proposed in this pull request?
This reverts commit 450b415.

### Why are the changes needed?
In apache#32888, shahidki31 change taskInfo.index to taskInfo.taskId. However, we generally use `index.attempt` or `taskId` to distinguish tasks within a stage, not `taskId.attempt`.
Thus apache#32888 was a wrong fix issue, we should revert it.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
origin test suites

Closes apache#35160 from stczwd/SPARK-37860.

Authored-by: stczwd <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 3d2fde5)
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit db1023c)
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants