Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Dec 7, 2023

What changes were proposed in this pull request?

This PR aims to log Spark HA recovery duration.

Why are the changes needed?

It will show the elapsed time like the following.

- 23/12/07 18:25:29 INFO Master: Recovery complete - resuming operations!
- 23/12/07 18:25:29 INFO Master: Recovery complete in 60.020s - resuming operations!

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CIs and manual review.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the CORE label Dec 7, 2023
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-46313][CORE] Log Spark HA recovery duration [SPARK-46313][CORE] Log Spark HA recovery duration Dec 7, 2023
schedule()
logInfo("Recovery complete - resuming operations!")
val timeTakenNs = System.nanoTime() - recoveryStartTimeNs
logInfo(s"Recovery complete in ${timeTakenNs / 1000000000d}s - resuming operations!")
Copy link
Member

Choose a reason for hiding this comment

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

0.043445167s may be less readable for humans. How about rounding to milliseconds?

TimeUnit.NANOSECONDS.toMillis(timeTakenNs) ms or TimeUnit.NANOSECONDS.toMillis(timeTakenNs) / 1000.0 s

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!

@dongjoon-hyun
Copy link
Member Author

To @yaooqinn , I addressed your comments and updated the PR description example.

Copy link
Member

@yaooqinn yaooqinn left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @dongjoon-hyun

@dongjoon-hyun
Copy link
Member Author

Thank you, @yaooqinn ! Merged to master.

dbatomic pushed a commit to dbatomic/spark that referenced this pull request Dec 11, 2023
### What changes were proposed in this pull request?

This PR aims to log `Spark HA` recovery duration.

### Why are the changes needed?

It will show the elapsed time like the following.
```
- 23/12/07 18:25:29 INFO Master: Recovery complete - resuming operations!
- 23/12/07 18:25:29 INFO Master: Recovery complete in 60.020s - resuming operations!
```

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

No.

### How was this patch tested?

Pass the CIs and manual review.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#44243 from dongjoon-hyun/SPARK-46313.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
szehon-ho pushed a commit to szehon-ho/spark that referenced this pull request Feb 7, 2024
### What changes were proposed in this pull request?

This PR aims to log `Spark HA` recovery duration.

### Why are the changes needed?

It will show the elapsed time like the following.
```
- 23/12/07 18:25:29 INFO Master: Recovery complete - resuming operations!
- 23/12/07 18:25:29 INFO Master: Recovery complete in 60.020s - resuming operations!
```

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

No.

### How was this patch tested?

Pass the CIs and manual review.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#44243 from dongjoon-hyun/SPARK-46313.

Authored-by: Dongjoon Hyun <[email protected]>
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants