Skip to content

[SPARK-35746][UI] Fix taskid in the stage page task event timeline #32888

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.

Sign up for GitHub

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

Jump to bottom
Closed
shahidki31 wants to merge 1 commit into from
Closed

[SPARK-35746][UI] Fix taskid in the stage page task event timeline #32888

shahidki31 wants to merge 1 commit into from

Conversation

@shahidki31
Copy link
Contributor

@shahidki31 shahidki31 commented Jun 12, 2021 •
edited
Loading

What changes were proposed in this pull request?

Task id is given incorrect in the timeline plot in Stage Page

Why are the changes needed?

Map event timeline plots to correct task
Before:
image

After
image

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually tested

@shahidki31 shahidki31 changed the title [SPARK-35746][UI] Fix taskid in stage page task event timeline [SPARK-35746][UI] Fix taskid in the stage page task event timeline Jun 12, 2021
@shahidki31
Copy link
Contributor Author

shahidki31 commented Jun 12, 2021

cc @srowen @maropu Kindly review

srowen
srowen approved these changes Jun 12, 2021
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending tests.

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Test build #139721 has finished for PR 32888 at commit 1c39d44.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shahidki31
Copy link
Contributor Author

shahidki31 commented Jun 12, 2021

Retest this please

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44246/

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44248/

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44246/

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44248/

@SparkQA
Copy link

SparkQA commented Jun 12, 2021

Test build #139723 has finished for PR 32888 at commit 1c39d44.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sarutak sarutak closed this in 450b415 Jun 12, 2021
sarutak pushed a commit that referenced this pull request Jun 12, 2021
@shahidki31 @sarutak
[SPARK-35746][UI] Fix taskid in the stage page task event timeline
78d3d0f 
### What changes were proposed in this pull request?
Task id is given incorrect in the timeline plot in Stage Page

### Why are the changes needed?
Map event timeline plots to correct task
**Before:**
![image](https://user-images.githubusercontent.com/23054875/121761077-81775800-cb4b-11eb-8ec6-ee71926a6549.png)

**After**
![image](https://user-images.githubusercontent.com/23054875/121761195-02ceea80-cb4c-11eb-8ce6-07bb1cca190e.png)
### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Manually tested

Closes #32888 from shahidki31/shahid/fixtaskid.

Authored-by: shahid <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 450b415)
Signed-off-by: Kousuke Saruta <[email protected]>
sarutak pushed a commit that referenced this pull request Jun 12, 2021
@shahidki31 @sarutak
[SPARK-35746][UI] Fix taskid in the stage page task event timeline
6597c3b 
### What changes were proposed in this pull request?
Task id is given incorrect in the timeline plot in Stage Page

### Why are the changes needed?
Map event timeline plots to correct task
**Before:**
![image](https://user-images.githubusercontent.com/23054875/121761077-81775800-cb4b-11eb-8ec6-ee71926a6549.png)

**After**
![image](https://user-images.githubusercontent.com/23054875/121761195-02ceea80-cb4c-11eb-8ce6-07bb1cca190e.png)
### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Manually tested

Closes #32888 from shahidki31/shahid/fixtaskid.

Authored-by: shahid <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 450b415)
Signed-off-by: Kousuke Saruta <[email protected]>
@sarutak
Copy link
Member

sarutak commented Jun 12, 2021

Merged to master, branch-3.1 and branch-3.0. Thanks!

@shahidki31
Copy link
Contributor Author

shahidki31 commented Jun 12, 2021

Thanks @sarutak , @srowen for the reviews

@shahidki31 shahidki31 deleted the shahid/fixtaskid branch June 12, 2021 09:58
flyrain pushed a commit to flyrain/spark that referenced this pull request Sep 21, 2021
@shahidki31 @dongjoon-hyun
[SPARK-35746][UI] Fix taskid in the stage page task event timeline
8866734 
### What changes were proposed in this pull request?
Task id is given incorrect in the timeline plot in Stage Page

### Why are the changes needed?
Map event timeline plots to correct task
**Before:**
![image](https://user-images.githubusercontent.com/23054875/121761077-81775800-cb4b-11eb-8ec6-ee71926a6549.png)

**After**
![image](https://user-images.githubusercontent.com/23054875/121761195-02ceea80-cb4c-11eb-8ce6-07bb1cca190e.png)
### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Manually tested

Closes apache#32888 from shahidki31/shahid/fixtaskid.

Authored-by: shahid <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 450b415)
Signed-off-by: Kousuke Saruta <[email protected]>
@jackylee-ch
Copy link
Contributor

jackylee-ch commented Jan 10, 2022 •
edited
Loading

Hm, why we change taskIndex.attemptId to taskId.attempt? @shahidki31 @srowen @sarutak
It seems we should either use Task 0 (attempt 0) or Task 100, not the Task 100 (attempt 0).

@jackylee-ch
Copy link
Contributor

jackylee-ch commented Jan 10, 2022

Shall we revert this？

This was referenced Jan 10, 2022
Closed
Closed
Closed
sarutak pushed a commit that referenced this pull request Jan 11, 2022
@sarutak
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
3d2fde5 
### 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]>
sarutak pushed a commit that referenced this pull request Jan 11, 2022
@sarutak
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
db1023c 
### 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
@sarutak
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
830d5b6 
### 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
@sarutak
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
755d11d 
### 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]>
fishcus pushed a commit to fishcus/spark that referenced this pull request Jan 12, 2022
@shahidki31 @fishcus
[SPARK-35746][UI] Fix taskid in the stage page task event timeline
f87ad51 
### What changes were proposed in this pull request?
Task id is given incorrect in the timeline plot in Stage Page

### Why are the changes needed?
Map event timeline plots to correct task
**Before:**
![image](https://user-images.githubusercontent.com/23054875/121761077-81775800-cb4b-11eb-8ec6-ee71926a6549.png)

**After**
![image](https://user-images.githubusercontent.com/23054875/121761195-02ceea80-cb4c-11eb-8ce6-07bb1cca190e.png)
### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Manually tested

Closes apache#32888 from shahidki31/shahid/fixtaskid.

Authored-by: shahid <[email protected]>
Signed-off-by: Kousuke Saruta <[email protected]>
(cherry picked from commit 450b415)
Signed-off-by: Kousuke Saruta <[email protected]>
fishcus pushed a commit to fishcus/spark that referenced this pull request Jan 12, 2022
@fishcus
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
dd437c0 
### 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
@dchvn
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
73ec7d7 
### 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
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
22b94ae 
### 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
@catalinii
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
410fa90 
### 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
@dongjoon-hyun
[SPARK-37860][UI] Fix taskindex in the stage page task event timeline
9647e1b 
### 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

Reviewers

@srowen srowen srowen approved these changes

@sarutak sarutak sarutak approved these changes

Assignees

No one assigned

Labels

CORE WEB UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@shahidki31 @SparkQA @sarutak @jackylee-ch @srowen