You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/01/08 06:09:52 UTC
[GitHub] [pulsar] Nicklee007 opened a new pull request #13678: release old bundle from ownership cache when operator split bundle
Nicklee007 opened a new pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678
Fixes #13677
Modifications
release the old bundle from ownership and temporary znode cache when we split the old bundle;
## Modifications
remove the old bundle when we split the bundle successed, and async release the temporary znode
## Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
no-need-doc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1007933383
@Nicklee007 Could you please add unit tests for this case ? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#discussion_r781738405
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
##########
@@ -496,6 +497,53 @@ public void testRemoveOwnershipAndSplitBundle() throws Exception {
}
}
+
+ @Test
+ public void testSplitBundleAndRemoveOldBundleFromOwnerShipCache() throws Exception {
+ OwnershipCache ownershipCache = spy(pulsar.getNamespaceService().getOwnershipCache());
+ doReturn(CompletableFuture.completedFuture(null)).when(ownershipCache).disableOwnership(any(NamespaceBundle.class));
+
+ Field ownership = NamespaceService.class.getDeclaredField("ownershipCache");
+ ownership.setAccessible(true);
+ ownership.set(pulsar.getNamespaceService(), ownershipCache);
+
+ NamespaceService namespaceService = pulsar.getNamespaceService();
+ NamespaceName nsname = NamespaceName.get("pulsar/global/ns1");
+ TopicName topicName = TopicName.get("persistent://pulsar/global/ns1/topic-1");
+ NamespaceBundles bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+
+
+ NamespaceBundle splitBundle1 = bundles.findBundle(topicName);
+ ownershipCache.tryAcquiringOwnership(splitBundle1);
+ CompletableFuture<Void> result1 = namespaceService.splitAndOwnBundle(splitBundle1, false, NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_ALGO);
+ try {
+ result1.get();
+ } catch (Exception e) {
+ fail("split bundle failed", e);
+ }
+ Awaitility.await().untilAsserted(()
+ -> assertNull(namespaceService.getOwnershipCache().getOwnedBundles().get(splitBundle1)));
+
+
+ //unload split
+ bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+ assertNotNull(bundles);
+ NamespaceBundle splitBundle2 = bundles.findBundle(topicName);
+ CompletableFuture<Void> result2 = namespaceService.splitAndOwnBundle(splitBundle2, true, NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_ALGO);
+ try {
+ result2.get();
+ } catch (Exception e) {
+ // make sure: NPE does not occur
+ fail("split bundle failed", e);
+ }
+ Awaitility.await().untilAsserted(()
+ -> assertNull(namespaceService.getOwnershipCache().getOwnedBundles().get(splitBundle2)));
+
Review comment:
unneeded space
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
##########
@@ -496,6 +497,53 @@ public void testRemoveOwnershipAndSplitBundle() throws Exception {
}
}
+
+ @Test
+ public void testSplitBundleAndRemoveOldBundleFromOwnerShipCache() throws Exception {
+ OwnershipCache ownershipCache = spy(pulsar.getNamespaceService().getOwnershipCache());
+ doReturn(CompletableFuture.completedFuture(null)).when(ownershipCache).disableOwnership(any(NamespaceBundle.class));
+
+ Field ownership = NamespaceService.class.getDeclaredField("ownershipCache");
+ ownership.setAccessible(true);
+ ownership.set(pulsar.getNamespaceService(), ownershipCache);
+
+ NamespaceService namespaceService = pulsar.getNamespaceService();
+ NamespaceName nsname = NamespaceName.get("pulsar/global/ns1");
+ TopicName topicName = TopicName.get("persistent://pulsar/global/ns1/topic-1");
+ NamespaceBundles bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+
+
+ NamespaceBundle splitBundle1 = bundles.findBundle(topicName);
+ ownershipCache.tryAcquiringOwnership(splitBundle1);
+ CompletableFuture<Void> result1 = namespaceService.splitAndOwnBundle(splitBundle1, false, NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_ALGO);
+ try {
+ result1.get();
+ } catch (Exception e) {
+ fail("split bundle failed", e);
+ }
+ Awaitility.await().untilAsserted(()
+ -> assertNull(namespaceService.getOwnershipCache().getOwnedBundles().get(splitBundle1)));
+
+
+ //unload split
+ bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+ assertNotNull(bundles);
+ NamespaceBundle splitBundle2 = bundles.findBundle(topicName);
+ CompletableFuture<Void> result2 = namespaceService.splitAndOwnBundle(splitBundle2, true, NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_ALGO);
+ try {
+ result2.get();
+ } catch (Exception e) {
+ // make sure: NPE does not occur
+ fail("split bundle failed", e);
+ }
+ Awaitility.await().untilAsserted(()
+ -> assertNull(namespaceService.getOwnershipCache().getOwnedBundles().get(splitBundle2)));
+
Review comment:
unneeded space line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a change in pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on a change in pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#discussion_r781738658
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
##########
@@ -496,6 +497,53 @@ public void testRemoveOwnershipAndSplitBundle() throws Exception {
}
}
+
+ @Test
+ public void testSplitBundleAndRemoveOldBundleFromOwnerShipCache() throws Exception {
+ OwnershipCache ownershipCache = spy(pulsar.getNamespaceService().getOwnershipCache());
+ doReturn(CompletableFuture.completedFuture(null)).when(ownershipCache).disableOwnership(any(NamespaceBundle.class));
+
+ Field ownership = NamespaceService.class.getDeclaredField("ownershipCache");
+ ownership.setAccessible(true);
+ ownership.set(pulsar.getNamespaceService(), ownershipCache);
+
+ NamespaceService namespaceService = pulsar.getNamespaceService();
+ NamespaceName nsname = NamespaceName.get("pulsar/global/ns1");
+ TopicName topicName = TopicName.get("persistent://pulsar/global/ns1/topic-1");
+ NamespaceBundles bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+
+
Review comment:
Suggest one space line instead of two
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 removed a comment on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Nicklee007 removed a comment on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1009570929
>
ok , I have add unit tests for this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on a change in pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on a change in pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#discussion_r781746847
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
##########
@@ -496,6 +497,53 @@ public void testRemoveOwnershipAndSplitBundle() throws Exception {
}
}
+
+ @Test
+ public void testSplitBundleAndRemoveOldBundleFromOwnerShipCache() throws Exception {
+ OwnershipCache ownershipCache = spy(pulsar.getNamespaceService().getOwnershipCache());
+ doReturn(CompletableFuture.completedFuture(null)).when(ownershipCache).disableOwnership(any(NamespaceBundle.class));
+
+ Field ownership = NamespaceService.class.getDeclaredField("ownershipCache");
+ ownership.setAccessible(true);
+ ownership.set(pulsar.getNamespaceService(), ownershipCache);
+
+ NamespaceService namespaceService = pulsar.getNamespaceService();
+ NamespaceName nsname = NamespaceName.get("pulsar/global/ns1");
+ TopicName topicName = TopicName.get("persistent://pulsar/global/ns1/topic-1");
+ NamespaceBundles bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+
+
Review comment:
> Suggest one space line instead of two
@Shoothzj Thank you for your correction，update it yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1007894021
@Nicklee007:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1007894021
@Nicklee007:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1009571375
>
@Shoothzj ok, I have add unit test for the case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Nicklee007 commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1009570929
>
ok , I have add unit tests for this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1007933383
@Nicklee007 Could you please add unit tests for this case ? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #13678: release old bundle from ownership cache when operator split bundle
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #13678:
URL: https://github.com/apache/pulsar/pull/13678#issuecomment-1008498761
@Nicklee007
You delete the backticks (``) so that Bot can not recognize the info and then labels the PR with `doc-info-missing`.
[x] `doc-required` ✅
[x] doc-required ❌
Instructions of doc labels: https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0/edit#bookmark=id.5d66olk7l4oz
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org