Skip to content
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

Configure split-bundle task interval different than loadReportGenerationTask-interval #183

Merged
merged 1 commit into from Jan 31, 2017

Conversation

rdhabalia
Copy link
Contributor

Motivation

If loadBalancerReportUpdateMaxIntervalMinutes is same as loadBalancerNamespaceBundleSplitIntervalMinutes (by default = 15 mins) then splitBundleTask may read stale List of Bundles and it will end up splitting invalid bundles which cause #172

Modifications

Keep different interval value for : loadBalancerReportUpdateMaxIntervalMinutes and loadBalancerNamespaceBundleSplitIntervalMinutes, to avoid stale-read of bundle data.

Result

It will fix #172

@rdhabalia rdhabalia added the type/bug The PR fixed a bug or issue reported a bug label Jan 31, 2017
@rdhabalia rdhabalia added this to the 1.16 milestone Jan 31, 2017
@rdhabalia rdhabalia self-assigned this Jan 31, 2017
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Is it possible to make it working for any config values? I haven't checked the code there but maybe there's a way to prevent both tasks from running st the same time.

@rdhabalia
Copy link
Contributor Author

Is it possible to make it working for any config values? I haven't checked the code there but maybe there's a way to prevent both tasks from running st the same time.

I think we can remove splitTaskConfiguration(loadBalancerNamespaceBundleSplitIntervalMinutes) instead keep its value as loadBalancerReportUpdateMaxIntervalMinutes + 5 because we have to make sure that splitBundle should run only after loadReport gets updated, otherwise there could be a possibility to read stale loadReport value.
Any thought?

@merlimat
Copy link
Contributor

Wouldn't it be easier to chain the 2 tasks in the same thread? Compute the load report and then check for bundles to be split.

Though, my concern is that the exception in #172 could happen anyway, cause a bundle could be unloaded after the report was generated. We should just ignore that bundle and move on at that point.

@rdhabalia
Copy link
Contributor Author

Make sense. I will make the change.

@rdhabalia rdhabalia force-pushed the split_load branch 2 times, most recently from 7a728cf to 5dfd223 Compare January 31, 2017 21:54
@rdhabalia
Copy link
Contributor Author

updated PR by merging loadReportGeneration and splitBundleTask.

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@merlimat merlimat merged commit 99f7e21 into apache:master Jan 31, 2017
@merlimat
Copy link
Contributor

@rdhabalia Anything else for 1.16 ?

@rdhabalia rdhabalia deleted the split_load branch February 1, 2017 22:06
sijie pushed a commit to sijie/pulsar that referenced this pull request Mar 4, 2018
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
* Support Pulsar 2.6.1

* Fix unit test error of testOffsetCommittedBacklogCleared
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to split namespace bundle
2 participants