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

Update bundle-cache on split-bundle and avoid disabling main bundle #822

Merged
merged 3 commits into from
Oct 13, 2017

Conversation

rdhabalia
Copy link
Contributor

@rdhabalia rdhabalia commented Oct 13, 2017

Motivation

As described at #821 : Lookup fails after bundle-split.
I am able to reproduce with 2 brokers and with SimpleLoadManagerImpl enabled.

  1. Broker-2 receives a lookup request for a brand-new namespace (which doesn't have local-policies), so, broker-2 creates local-policies, puts bundle data into bundlesCache, and assigns bundle to Broker-1.
  2. Now, Broker-1 splits bundle, disable original bundle and updates local-policies.
  3. Broker-2 doesn't know about newly split bundle and original bundle is disable. So, any lookup request for this bundle on broker-2 will receive above error.

Modifications

  1. Set the watch for local-policies and invalidate BundleCache when watch triggers.
  2. Don't disable original bundle and let it be enabled so, if any other broker which didn't receive watch, can still think that bundle is enabled and redirect the request to original broker. It will do two things:
  • Lookup will not fail on broker which didn't receive request and it redirects to original broker
  • Original broker knows in which bundle that topic falls and serves request accordingly.

Result

It will fix lookup-failure for split-bundle.

@rdhabalia rdhabalia added the type/bug The PR fixed a bug or issue reported a bug label Oct 13, 2017
@rdhabalia rdhabalia added this to the 1.21.0-incubating milestone Oct 13, 2017
@rdhabalia rdhabalia self-assigned this Oct 13, 2017
try {
ownershipCache.disableOwnership(bundle);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed, or just a safety measure to have the parent bundle to be still "active" in case other brokers don't have the local cache updated yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that protection may need while broker is refreshing cache (invalidate old bundle and assign topic-ownership to new bundles). So, I will add that protection again by disabling namespace in memory only but other broker can still redirect to this broker.

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.

👍

@apache apache deleted a comment from merlimat Oct 13, 2017
@merlimat merlimat merged commit f551c4f into apache:master Oct 13, 2017
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.

None yet

2 participants