Skip to content

Remove wrong TimeSliceAllocator#11569

Merged
lancelly merged 1 commit intomasterfrom
RemoveTimeSliceAllocator
Nov 21, 2023
Merged

Remove wrong TimeSliceAllocator#11569
lancelly merged 1 commit intomasterfrom
RemoveTimeSliceAllocator

Conversation

@JackieTien97
Copy link
Copy Markdown
Contributor

We never need to split the time slice for each operator in one DriverTask because we're using pull-based execution model, even if one child consume all the time slice of one dirver, its parent should detect that and then stop current run immediately instead of keeping runing next child operator.

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Comment on lines +99 to +100
public static void setMaxRunTime(Duration maxRunTime) {
OperatorContext.maxRunTime = maxRunTime;
Copy link
Copy Markdown
Contributor

@lancelly lancelly Nov 18, 2023

Choose a reason for hiding this comment

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

why not remove this method since it is now only used in tests and ClientRpcServiceImpl? I think there's no need to call this method both in tests and ClientRpcServiceImpl, too.
截屏2023-11-18 10 37 28

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, I wanna keep this method for future usage like we want to change the time slice for running DN without restarting it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I'm not mistaken, changing the time slice length without restarting should be done through calling a Session interface or executing a SQL . However, setting OperatorMaxRuntime doesn't seem to work, and it seems that what should be changed is the value in the Config, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but in current situation, each operator's max run time is got from OperatorContext. And calling a Session interface or executing a SQL is to change the value in OperatorContext.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see.

Copy link
Copy Markdown
Contributor

@lancelly lancelly left a comment

Choose a reason for hiding this comment

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

LGTM

@lancelly lancelly merged commit 0acd405 into master Nov 21, 2023
@JackieTien97 JackieTien97 deleted the RemoveTimeSliceAllocator branch November 21, 2023 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants