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 2021/09/25 22:53:37 UTC
[GitHub] [pulsar] merlimat opened a new pull request #12193: Fixed counter base path getting deleted
merlimat opened a new pull request #12193:
URL: https://github.com/apache/pulsar/pull/12193
### Motivation
In #11925, we switched to use ZK container type for intermediate nodes so that they will be automatically deleted when empty. This kind of broke the counter logic because it's relying on the z-node version of the base path: if the base path goes away and it gets recreated, the next counter value is going to be reset to 0.
### Modifications
Pre-create the base path as a non-ephemeral, non-container key that will not be deleted.
--
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] eolivelli commented on a change in pull request #12193: Fixed counter base path getting deleted
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12193:
URL: https://github.com/apache/pulsar/pull/12193#discussion_r716214376
##########
File path: pulsar-metadata/src/test/java/org/apache/pulsar/metadata/CounterTest.java
##########
@@ -51,4 +51,40 @@ public void basicTest(String provider, Supplier<String> urlSupplier) throws Exce
long l4 = cs1.getNextCounterValue("/my/path").join();
assertNotEquals(l3, l4);
}
+
+ @Test(dataProvider = "impl")
+ public void testCounterDoesNotAutoReset(String provider, Supplier<String> urlSupplier) throws Exception {
+ if (provider.equals("Memory")) {
+ // Test doesn't make sense for local memory since we're testing across different instances
+ return;
+ }
+
+ MetadataStoreExtended store1 = MetadataStoreExtended.create(urlSupplier.get(),
+ MetadataStoreConfig.builder().build());
+
+ CoordinationService cs1 = new CoordinationServiceImpl(store1);
+
+ long l1 = cs1.getNextCounterValue("/my/path").join();
+ long l2 = cs1.getNextCounterValue("/my/path").join();
+ long l3 = cs1.getNextCounterValue("/my/path").join();
+
+ assertNotEquals(l1, l2);
+ assertNotEquals(l2, l3);
+
+ cs1.close();
+ store1.close();;
Review comment:
Nit: remove extra semi colon
##########
File path: pulsar-metadata/src/test/java/org/apache/pulsar/metadata/CounterTest.java
##########
@@ -51,4 +51,40 @@ public void basicTest(String provider, Supplier<String> urlSupplier) throws Exce
long l4 = cs1.getNextCounterValue("/my/path").join();
assertNotEquals(l3, l4);
}
+
+ @Test(dataProvider = "impl")
+ public void testCounterDoesNotAutoReset(String provider, Supplier<String> urlSupplier) throws Exception {
+ if (provider.equals("Memory")) {
+ // Test doesn't make sense for local memory since we're testing across different instances
+ return;
+ }
+
+ MetadataStoreExtended store1 = MetadataStoreExtended.create(urlSupplier.get(),
+ MetadataStoreConfig.builder().build());
+
+ CoordinationService cs1 = new CoordinationServiceImpl(store1);
+
+ long l1 = cs1.getNextCounterValue("/my/path").join();
+ long l2 = cs1.getNextCounterValue("/my/path").join();
+ long l3 = cs1.getNextCounterValue("/my/path").join();
+
+ assertNotEquals(l1, l2);
+ assertNotEquals(l2, l3);
+
+ cs1.close();
+ store1.close();;
+
+ // Delete all the empty container nodes
+ zks.checkContainers();
+
+ MetadataStoreExtended store2 = MetadataStoreExtended.create(urlSupplier.get(),
Review comment:
Oh, right!
So we can just test that it works well
--
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] merlimat commented on a change in pull request #12193: Fixed counter base path getting deleted
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12193:
URL: https://github.com/apache/pulsar/pull/12193#discussion_r716207869
##########
File path: pulsar-metadata/src/test/java/org/apache/pulsar/metadata/CounterTest.java
##########
@@ -51,4 +51,40 @@ public void basicTest(String provider, Supplier<String> urlSupplier) throws Exce
long l4 = cs1.getNextCounterValue("/my/path").join();
assertNotEquals(l3, l4);
}
+
+ @Test(dataProvider = "impl")
+ public void testCounterDoesNotAutoReset(String provider, Supplier<String> urlSupplier) throws Exception {
+ if (provider.equals("Memory")) {
+ // Test doesn't make sense for local memory since we're testing across different instances
+ return;
+ }
+
+ MetadataStoreExtended store1 = MetadataStoreExtended.create(urlSupplier.get(),
+ MetadataStoreConfig.builder().build());
+
+ CoordinationService cs1 = new CoordinationServiceImpl(store1);
+
+ long l1 = cs1.getNextCounterValue("/my/path").join();
+ long l2 = cs1.getNextCounterValue("/my/path").join();
+ long l3 = cs1.getNextCounterValue("/my/path").join();
+
+ assertNotEquals(l1, l2);
+ assertNotEquals(l2, l3);
+
+ cs1.close();
+ store1.close();;
+
+ // Delete all the empty container nodes
+ zks.checkContainers();
+
+ MetadataStoreExtended store2 = MetadataStoreExtended.create(urlSupplier.get(),
Review comment:
The point of the change is for the parent z-node to *not* be deleted :)
--
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] eolivelli commented on a change in pull request #12193: Fixed counter base path getting deleted
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12193:
URL: https://github.com/apache/pulsar/pull/12193#discussion_r716152327
##########
File path: pulsar-metadata/src/test/java/org/apache/pulsar/metadata/CounterTest.java
##########
@@ -51,4 +51,40 @@ public void basicTest(String provider, Supplier<String> urlSupplier) throws Exce
long l4 = cs1.getNextCounterValue("/my/path").join();
assertNotEquals(l3, l4);
}
+
+ @Test(dataProvider = "impl")
+ public void testCounterDoesNotAutoReset(String provider, Supplier<String> urlSupplier) throws Exception {
+ if (provider.equals("Memory")) {
+ // Test doesn't make sense for local memory since we're testing across different instances
+ return;
+ }
+
+ MetadataStoreExtended store1 = MetadataStoreExtended.create(urlSupplier.get(),
+ MetadataStoreConfig.builder().build());
+
+ CoordinationService cs1 = new CoordinationServiceImpl(store1);
+
+ long l1 = cs1.getNextCounterValue("/my/path").join();
+ long l2 = cs1.getNextCounterValue("/my/path").join();
+ long l3 = cs1.getNextCounterValue("/my/path").join();
+
+ assertNotEquals(l1, l2);
+ assertNotEquals(l2, l3);
+
+ cs1.close();
+ store1.close();;
+
+ // Delete all the empty container nodes
+ zks.checkContainers();
+
+ MetadataStoreExtended store2 = MetadataStoreExtended.create(urlSupplier.get(),
Review comment:
Can we validate that the znode has been really deleted?
--
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] merlimat merged pull request #12193: Fixed counter base path getting deleted
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #12193:
URL: https://github.com/apache/pulsar/pull/12193
--
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