Skip to content

Conversation

jshmchenxi
Copy link
Contributor

@jshmchenxi jshmchenxi commented Sep 2, 2024

-> This is the first commit for fixing SPARK-49479 in branch 3.5.
Second commit for fixing SPARK-49479 in branch 3.5: #47945
Third commit for fixing SPARK-49479 in branch 3.5: #47956

What changes were proposed in this pull request?

This PR propose to replace Timer with single thread scheduled executor for ConsoleProgressBar.

Why are the changes needed?

The javadoc recommends ScheduledThreadPoolExecutor instead of Timer.
屏幕快照 2024-01-12 下午12 47 57

This change based on the following two points.
System time sensitivity

Timer scheduling is based on the absolute time of the operating system and is sensitive to the operating system's time. Once the operating system's time changes, Timer scheduling is no longer precise.
The scheduled Thread Pool Executor scheduling is based on relative time and is not affected by changes in operating system time.

Are anomalies captured

Timer does not capture exceptions thrown by Timer Tasks, and in addition, Timer is single threaded. Once a scheduling task encounters an exception, the entire thread will terminate and other tasks that need to be scheduled will no longer be executed.
The scheduled Thread Pool Executor implements scheduling functions based on a thread pool. After a task throws an exception, other tasks can still execute normally.

Does this PR introduce any user-facing change?

'No'.

How was this patch tested?

Manual tests.

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

'No'.

…or for ConsoleProgressBar

### What changes were proposed in this pull request?
This PR propose to replace `Timer` with single thread scheduled executor for `ConsoleProgressBar`.

### Why are the changes needed?
The javadoc recommends `ScheduledThreadPoolExecutor` instead of `Timer`.
![屏幕快照 2024-01-12 下午12 47 57](https://github.com/apache/spark/assets/8486025/4fc5ed61-6bb9-4768-915a-ad919a067d04)

This change based on the following two points.
**System time sensitivity**

Timer scheduling is based on the absolute time of the operating system and is sensitive to the operating system's time. Once the operating system's time changes, Timer scheduling is no longer precise.
The scheduled Thread Pool Executor scheduling is based on relative time and is not affected by changes in operating system time.

**Are anomalies captured**

Timer does not capture exceptions thrown by Timer Tasks, and in addition, Timer is single threaded. Once a scheduling task encounters an exception, the entire thread will terminate and other tasks that need to be scheduled will no longer be executed.
The scheduled Thread Pool Executor implements scheduling functions based on a thread pool. After a task throws an exception, other tasks can still execute normally.

### Does this PR introduce _any_ user-facing change?
'No'.

### How was this patch tested?
Manual tests.

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

Closes apache#44701 from beliefer/replace-timer-with-scheduled-executor.

Authored-by: beliefer <beliefer@163.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit fc66576)
@HyukjinKwon
Copy link
Member

cc @beliefer

Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jan 27, 2025
@github-actions github-actions bot closed this Jan 28, 2025
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.

3 participants