-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feature:support random deferred compression #4011
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
base: 2.x
Are you sure you want to change the base?
Conversation
|
@ |
vy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OpenZYK, I think the suggested feature sounds legitimate. I've skimmed through the changes, and dropped some remarks. Would you also mind providing a test and a changelog entry, please?
| * Wrapper action that schedules compression for defferred execution. | ||
| * This action wraps another action and schedules it to be executed | ||
| * after a random defer within a specified time window. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Wrapper action that schedules compression for defferred execution. | |
| * This action wraps another action and schedules it to be executed | |
| * after a random defer within a specified time window. | |
| * Decorates an {@link Action} to execute it after a certain amount of delay. |
| private boolean interrupted = false; | ||
|
|
||
| /** | ||
| * Creates a new DelayedCompressionAction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Creates a new DelayedCompressionAction. | |
| * Creates a new instance. |
| /** | ||
| * Creates a new DelayedCompressionAction. | ||
| * | ||
| * @param originalAction the action to be executed with defer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @param originalAction the action to be executed with defer | |
| * @param originalAction the action to be executed with delay |
| * This action wraps another action and schedules it to be executed | ||
| * after a random defer within a specified time window. | ||
| */ | ||
| public class DeferredCompressionAction implements Action { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your earlier attempt, DelayedCompressionAction, fits the bill better. AFAICT, deferred waits for a condition, which does not necessarily needs to be a time, whereas delay makes it clear the involvement of time.
Once you implement this change, please update all the usages of [Dd]efer in this PR.
| /** | ||
| * Executes the action with a defer using sleep. | ||
| * | ||
| * @return true if the action was successfully executed | ||
| * @throws IOException if an error occurs during execution | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | |
| * Executes the action with a defer using sleep. | |
| * | |
| * @return true if the action was successfully executed | |
| * @throws IOException if an error occurs during execution | |
| */ |
| */ | ||
| @Override | ||
| public boolean execute() throws IOException { | ||
| if (interrupted) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about complete == true?
| * @param originalAction the action to be executed with defer | ||
| * @param maxDeferSeconds the maximum defer in seconds (0 means immediate execution) | ||
| */ | ||
| public DeferredCompressionAction(Action originalAction, int maxDeferSeconds) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Shouldn't we be receiving a range instead of an upper bound?
|
@OpenZYK, TimeBasedTriggeringPolicy receives a |
At 0 o 'clock every day, a large number of machines begin to compress the log files of the previous day, resulting in the sharp rise of the disk IO of the container, which brings risk to the stability of the cloud disk. Now, the compression random delay x seconds is introduced to reduce the concentration of the compression trigger time and reduce the pressure on the disk.