[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378147169 seems CI build error could fix with #1489, may need retry after it merged This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378147169 seems CI build error comes from #1489, may need retry after it merged This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] coderfender opened a new issue #1490: Homebrew Package for apache-oulsar
coderfender opened a new issue #1490: Homebrew Package for apache-oulsar URL: https://github.com/apache/incubator-pulsar/issues/1490 Expected behavior Tell us what should happen Actual behavior Tell us what happens instead Steps to reproduce How can we reproduce the issue System configuration **Pulsar version**: x.y This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
sijie commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378153429 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378147169 seems CI build error comes from #1489 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378147169 build error comes from #1489 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1490: Linux/Homebrew Package for apache-oulsar
merlimat commented on issue #1490: Linux/Homebrew Package for apache-oulsar URL: https://github.com/apache/incubator-pulsar/issues/1490#issuecomment-378306048 @coderfender Nice! For homebrew there was some work done here https://github.com/streamlio/homebrew-formulae/blob/master/pulsar.rb by @aahmed-se It would be nice to have all packaging in same repo. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1271: Message deduplication documentation
merlimat commented on issue #1271: Message deduplication documentation URL: https://github.com/apache/incubator-pulsar/pull/1271#issuecomment-378319204 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1271: Message deduplication documentation
merlimat commented on issue #1271: Message deduplication documentation URL: https://github.com/apache/incubator-pulsar/pull/1271#issuecomment-378319204 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378376262 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378376262 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jai1 commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
jai1 commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378402380 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378407272 > I guess we will always run into these issues as long as we keep pointing to SNAPSHOT jar. > Maybe we should eventually stop using BK SNAPSHOT jar. Yes, once we're sure BK 4.7 is ready and has all the pieces we need for Pulsar, a release will be made and we'll switch to that release. That should happen soon. There were a lot of thing that we had to adjust on both Pulsar and BK side to make the transition to 4.7, it wasn't just possible to make these changes in a branch and sync with master. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on issue #1492: Removed Pulsar Functions from AdminTool
sijie commented on issue #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#issuecomment-378441045 @jai1 I think it is #1327 broke the admin tool. so a better fix is to fix PulsarAdminTool to construct PulsarAdminWithFunctions. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on issue #1440: Update default values for a few publisher settings
sijie commented on issue #1440: Update default values for a few publisher settings URL: https://github.com/apache/incubator-pulsar/pull/1440#issuecomment-378443928 @merlimat - rebased to latest master and updated javadocs This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[incubator-pulsar] branch master updated: Refactored to reflect changes in BK for OrderedExecutor (#1489)
This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pulsar.git The following commit(s) were added to refs/heads/master by this push: new d309b2c Refactored to reflect changes in BK for OrderedExecutor (#1489) d309b2c is described below commit d309b2cb2f766151dc64861ba15e6dc71fbe8d8f Author: Matteo MerliAuthorDate: Tue Apr 3 14:48:46 2018 -0700 Refactored to reflect changes in BK for OrderedExecutor (#1489) * Refactored to reflect changes in BK for OrderedExecutor * Reverted back part of last change * Fixed test --- .../bookkeeper/mledger/impl/EntryCacheImpl.java| 4 +- .../bookkeeper/mledger/impl/EntryCacheManager.java | 2 +- .../bookkeeper/mledger/impl/ManagedCursorImpl.java | 6 ++- .../mledger/impl/ManagedLedgerFactoryImpl.java | 21 +- .../bookkeeper/mledger/impl/ManagedLedgerImpl.java | 45 ++-- .../mledger/impl/MetaStoreImplZookeeper.java | 36 +--- .../apache/bookkeeper/mledger/impl/OpAddEntry.java | 4 +- .../bookkeeper/mledger/impl/OpReadEntry.java | 6 +-- .../mledger/impl/EntryCacheManagerTest.java| 6 +-- .../mledger/impl/ManagedLedgerMBeanTest.java | 8 ++-- .../org/apache/pulsar/broker/PulsarService.java| 9 ++-- .../pulsar/broker/namespace/NamespaceService.java | 4 +- .../pulsar/broker/service/BrokerService.java | 20 - .../apache/pulsar/broker/service/ServerCnx.java| 2 +- .../service/nonpersistent/NonPersistentTopic.java | 9 ++-- .../service/persistent/MessageDeduplication.java | 2 +- .../PersistentDispatcherMultipleConsumers.java | 2 +- .../broker/auth/SameThreadOrderedSafeExecutor.java | 14 +++ .../broker/cache/ResourceQuotaCacheTest.java | 10 + .../broker/namespace/OwnershipCacheTest.java | 11 ++--- .../apache/pulsar/client/impl/ConsumerImpl.java| 2 +- .../discovery/service/BrokerDiscoveryProvider.java | 1 - .../service/web/ZookeeperCacheLoader.java | 10 + .../proxy/server/util/ZookeeperCacheLoader.java| 10 + .../pulsar/zookeeper/GlobalZooKeeperCache.java | 6 +-- .../pulsar/zookeeper/LocalZooKeeperCache.java | 9 ++-- .../apache/pulsar/zookeeper/ZooKeeperCache.java| 49 ++ .../zookeeper/ZookeeperBkClientFactoryImpl.java| 12 +++--- .../pulsar/zookeeper/ZookeeperCacheTest.java | 12 +++--- 29 files changed, 152 insertions(+), 180 deletions(-) diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java index f267a6c..60ba634 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java @@ -188,7 +188,7 @@ public class EntryCacheImpl implements EntryCache { manager.mlFactoryMBean.recordCacheMiss(1, returnEntry.getLength()); ml.mbean.addReadEntriesSample(1, returnEntry.getLength()); -ml.getExecutor().submitOrdered(ml.getName(), safeRun(() -> { +ml.getExecutor().executeOrdered(ml.getName(), safeRun(() -> { callback.readEntryComplete(returnEntry, obj); })); } else { @@ -254,7 +254,7 @@ public class EntryCacheImpl implements EntryCache { checkNotNull(ml.getName()); checkNotNull(ml.getExecutor()); -ml.getExecutor().submitOrdered(ml.getName(), safeRun(() -> { +ml.getExecutor().executeOrdered(ml.getName(), safeRun(() -> { // We got the entries, we need to transform them to a List<> type long totalSize = 0; final List entriesToReturn = Lists.newArrayListWithExpectedSize(entriesToRead); diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java index 7faa18c..262cbeb 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java @@ -109,7 +109,7 @@ public class EntryCacheManager { // Trigger a single eviction in background. While the eviction is running we stop inserting entries in the cache if (currentSize > evictionTriggerThreshold && evictionInProgress.compareAndSet(false, true)) { -mlFactory.executor.execute(safeRun(() -> { +mlFactory.scheduledExecutor.execute(safeRun(() -> { // Trigger a new cache eviction cycle to bring the used memory below the
[GitHub] merlimat closed pull request #1489: Refactored to reflect changes in BK for OrderedExecutor
merlimat closed pull request #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java index f267a6c06..60ba634e1 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheImpl.java @@ -188,7 +188,7 @@ public void asyncReadEntry(LedgerHandle lh, PositionImpl position, final ReadEnt manager.mlFactoryMBean.recordCacheMiss(1, returnEntry.getLength()); ml.mbean.addReadEntriesSample(1, returnEntry.getLength()); -ml.getExecutor().submitOrdered(ml.getName(), safeRun(() -> { +ml.getExecutor().executeOrdered(ml.getName(), safeRun(() -> { callback.readEntryComplete(returnEntry, obj); })); } else { @@ -254,7 +254,7 @@ public void asyncReadEntry(LedgerHandle lh, long firstEntry, long lastEntry, boo checkNotNull(ml.getName()); checkNotNull(ml.getExecutor()); -ml.getExecutor().submitOrdered(ml.getName(), safeRun(() -> { +ml.getExecutor().executeOrdered(ml.getName(), safeRun(() -> { // We got the entries, we need to transform them to a List<> type long totalSize = 0; final List entriesToReturn = Lists.newArrayListWithExpectedSize(entriesToRead); diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java index 7faa18c8e..262cbeb4c 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/EntryCacheManager.java @@ -109,7 +109,7 @@ boolean hasSpaceInCache() { // Trigger a single eviction in background. While the eviction is running we stop inserting entries in the cache if (currentSize > evictionTriggerThreshold && evictionInProgress.compareAndSet(false, true)) { -mlFactory.executor.execute(safeRun(() -> { +mlFactory.scheduledExecutor.execute(safeRun(() -> { // Trigger a new cache eviction cycle to bring the used memory below the cacheEvictionWatermark // percentage limit long sizeToEvict = currentSize - (long) (maxSize * cacheEvictionWatermak); diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java index 29fe0a724..a2be71d15 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java @@ -37,6 +37,7 @@ import com.google.common.collect.TreeRangeSet; import com.google.common.util.concurrent.RateLimiter; import com.google.protobuf.InvalidProtocolBufferException; + import java.util.ArrayDeque; import java.util.Collections; import java.util.List; @@ -51,6 +52,7 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; + import org.apache.bookkeeper.client.AsyncCallback.CloseCallback; import org.apache.bookkeeper.client.AsyncCallback.DeleteCallback; import org.apache.bookkeeper.client.BKException; @@ -847,7 +849,7 @@ public void asyncResetCursor(Position newPos, AsyncCallbacks.ResetCursorCallback final PositionImpl newPosition = (PositionImpl) newPos; // order trim and reset operations on a ledger -ledger.getExecutor().submitOrdered(ledger.getName(), safeRun(() -> { +ledger.getExecutor().executeOrdered(ledger.getName(), safeRun(() -> { if (ledger.isValidPosition(newPosition) || newPosition.equals(PositionImpl.earliest) || newPosition.equals(PositionImpl.latest)) { internalResetCursor(newPosition, callback); @@ -1923,7 +1925,7 @@ void createNewMetadataLedger(final VoidCallback callback) { bookkeeper.asyncCreateLedger(config.getMetadataEnsemblesize(), config.getMetadataWriteQuorumSize(), config.getMetadataAckQuorumSize(), config.getDigestType(), config.getPassword(), (rc, lh, ctx) -> { -
[GitHub] merlimat opened a new pull request #1491: Fixed test_producer_send_async in Python
merlimat opened a new pull request #1491: Fixed test_producer_send_async in Python URL: https://github.com/apache/incubator-pulsar/pull/1491 Seen a couple of times: ``` == FAIL: test_producer_send_async (__main__.PulsarTest) -- Traceback (most recent call last): File "pulsar_test.py", line 91, in test_producer_send_async self.assertEqual(len(sent_messages), 3) AssertionError: 0 != 3 ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378437218 Seems there was no conflict, lt should be well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack opened a new issue #1493: bundle balance need also consider in-progress bundles
zhaijack opened a new issue #1493: bundle balance need also consider in-progress bundles URL: https://github.com/apache/incubator-pulsar/issues/1493 If a lot of bundles assignment happens at the same time, besides the history bundle assignment, the in-pregress bundles assignment may also be a consideration. So it could avoid assign a lot of bundles to a candidate broker. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1381: Schema registry 4/N
merlimat commented on issue #1381: Schema registry 4/N URL: https://github.com/apache/incubator-pulsar/pull/1381#issuecomment-378411591 @mgodave please merge with master again and it should go through CI this time. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1488: Load manager should offload multiple bundles when overloaded
merlimat commented on issue #1488: Load manager should offload multiple bundles when overloaded URL: https://github.com/apache/incubator-pulsar/pull/1488#issuecomment-378418126 @sijie @rdhabalia Updated This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor
merlimat commented on issue #1489: Refactored to reflect changes in BK for OrderedExecutor URL: https://github.com/apache/incubator-pulsar/pull/1489#issuecomment-378408950 > can we add timestamp snapshot version for BK. We can, though I don't think it needs to be part of this PR This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie closed pull request #1491: Fixed test_producer_send_async in Python
sijie closed pull request #1491: Fixed test_producer_send_async in Python URL: https://github.com/apache/incubator-pulsar/pull/1491 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/pulsar-client-cpp/python/pulsar_test.py b/pulsar-client-cpp/python/pulsar_test.py index 4d3e0adaf..f35736dbc 100755 --- a/pulsar-client-cpp/python/pulsar_test.py +++ b/pulsar-client-cpp/python/pulsar_test.py @@ -87,7 +87,10 @@ def send_callback(producer, msg): producer.send_async('hello', send_callback) producer.send_async('hello', send_callback) -time.sleep(0.1) +i = 0 +while len(sent_messages) < 3 and i < 100: +time.sleep(0.1) +i += 1 self.assertEqual(len(sent_messages), 3) client.close() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[incubator-pulsar] branch master updated: Fixed test_producer_send_async in Python (#1491)
This is an automated email from the ASF dual-hosted git repository. sijie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pulsar.git The following commit(s) were added to refs/heads/master by this push: new b052669 Fixed test_producer_send_async in Python (#1491) b052669 is described below commit b0526695c5c08a42bd10671c8a344eb44f0116b3 Author: Matteo MerliAuthorDate: Tue Apr 3 16:15:21 2018 -0700 Fixed test_producer_send_async in Python (#1491) --- pulsar-client-cpp/python/pulsar_test.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pulsar-client-cpp/python/pulsar_test.py b/pulsar-client-cpp/python/pulsar_test.py index 4d3e0ad..f35736d 100755 --- a/pulsar-client-cpp/python/pulsar_test.py +++ b/pulsar-client-cpp/python/pulsar_test.py @@ -87,7 +87,10 @@ class PulsarTest(TestCase): producer.send_async('hello', send_callback) producer.send_async('hello', send_callback) -time.sleep(0.1) +i = 0 +while len(sent_messages) < 3 and i < 100: +time.sleep(0.1) +i += 1 self.assertEqual(len(sent_messages), 3) client.close() -- To stop receiving notification emails like this one, please contact si...@apache.org.
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378434122 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jai1 opened a new pull request #1492: Removed Pulsar Functions from AdminTool
jai1 opened a new pull request #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492 Currently in master: ``` ./bin/pulsar-admin class java.lang.reflect.InvocationTargetException: null ``` Reason is that in `CmdFunctions.java:639` we expect the admin to be an instance of `PulsarAdminWithFunctions` while `PulsarAdminBuilderImpl.build()` builds an instance `PulsarAdmin` I tried building `PulsarAdminWithFunctions` in `PulsarAdminBuilderImpl` instead of `PulsarAdmin` but ran into lots of errors so I just commented out adding pulsar functions to `PulsarAdminTool`, in order to get master working since I need to develop on Admin Tools. I believe the problematic PR is https://github.com/apache/incubator-pulsar/commit/6230ab4542c733f3390bd7ae7d4d6b0d5a661fb1 but can't say for sure since we use SNAPSHOT bookkeeper versions now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jai1 commented on issue #1492: Removed Pulsar Functions from AdminTool
jai1 commented on issue #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#issuecomment-378469509 @sijie You are right #1327 broke the pulsar-admin Even after breaking my head for 2 hours I couldn't figure out a way of resolving the cycling dependency b/w pulsar-functions, pulsar-admin and pulsar-client This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365#issuecomment-378468333 Seems still some test error, will fix it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jai1 opened a new pull request #1494: In Namespace.addClusters and Properties.addRoles changed dataType to Set
jai1 opened a new pull request #1494: In Namespace.addClusters and Properties.addRoles changed dataType to Set URL: https://github.com/apache/incubator-pulsar/pull/1494 In a recent production incident, we noticed duplicate cluster names in Zookeeper on Namespace policies. I have fixed the admin commands and broker side admin classed to accept a set of cluster names and roles. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on issue #1492: Removed Pulsar Functions from AdminTool
sijie commented on issue #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#issuecomment-378470439 Let me work on that This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1059: Issue 1014: Rename "global zookeeper" to "configuration-store"(change in code, conf and cli)
zhaijack commented on issue #1059: Issue 1014: Rename "global zookeeper" to "configuration-store"(change in code, conf and cli) URL: https://github.com/apache/incubator-pulsar/pull/1059#issuecomment-378083616 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on issue #1059: Issue 1014: Rename "global zookeeper" to "configuration-store"(change in code, conf and cli)
zhaijack commented on issue #1059: Issue 1014: Rename "global zookeeper" to "configuration-store"(change in code, conf and cli) URL: https://github.com/apache/incubator-pulsar/pull/1059#issuecomment-378470938 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jai1 commented on issue #1492: Removed Pulsar Functions from AdminTool
jai1 commented on issue #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#issuecomment-378478744 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack commented on a change in pull request #1488: Load manager should offload multiple bundles when overloaded
zhaijack commented on a change in pull request #1488: Load manager should offload multiple bundles when overloaded URL: https://github.com/apache/incubator-pulsar/pull/1488#discussion_r179025858 ## File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/OverloadShedder.java ## @@ -40,73 +44,89 @@ * rate that has not been recently unloaded. */ public class OverloadShedder implements LoadSheddingStrategy { + private static final Logger log = LoggerFactory.getLogger(OverloadShedder.class); -private MapselectedBundlesCache; -/** - * Create an OverloadShedder with the service configuration. - * - * @param conf - *Service configuration to create from. - */ -public OverloadShedder(final ServiceConfiguration conf) { -selectedBundlesCache = new HashMap<>(); -} +private final Multimap selectedBundlesCache = ArrayListMultimap.create(); + +private final static double ADDITIONAL_THRESHOLD_PERCENT_MARGIN = 0.05; /** - * Attempt to shed one bundle off every broker which is overloaded. + * Attempt to shed some bundles off every broker which is overloaded. * * @param loadData *The load data to used to make the unloading decision. * @param conf *The service configuration. * @return A map from bundles to unload to the brokers on which they are loaded. */ -public Map findBundlesForUnloading(final LoadData loadData, final ServiceConfiguration conf) { +public Multimap findBundlesForUnloading(final LoadData loadData, final ServiceConfiguration conf) { selectedBundlesCache.clear(); final double overloadThreshold = conf.getLoadBalancerBrokerOverloadedThresholdPercentage() / 100.0; final Map recentlyUnloadedBundles = loadData.getRecentlyUnloadedBundles(); Review comment: Is it OK to avoid shed bundle, if all brokers are overload here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool
sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#discussion_r179026165 ## File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java ## @@ -312,6 +313,13 @@ public BrokerStats brokerStats() { public String getServiceUrl() { return serviceUrl; } + +/** + * @return the client Configuration Data that is being used + */ +public ClientConfigurationData getClientConfigData() { + return clientConfigData; + } Review comment: nit: spaces ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool
sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#discussion_r179026091 ## File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java ## @@ -103,8 +103,9 @@ public static PulsarAdminBuilder builder() { return new PulsarAdminBuilderImpl(); } -public PulsarAdmin(String serviceUrl, ClientConfigurationData pulsarConfig) throws PulsarClientException { -this.auth = pulsarConfig != null ? pulsarConfig.getAuthentication() : new AuthenticationDisabled(); +public PulsarAdmin(String serviceUrl, ClientConfigurationData clientConfigData) throws PulsarClientException { + this.clientConfigData = clientConfigData; Review comment: nit: wrong indent This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool
sijie commented on a change in pull request #1492: Removed Pulsar Functions from AdminTool URL: https://github.com/apache/incubator-pulsar/pull/1492#discussion_r179026053 ## File path: pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java ## @@ -71,7 +71,7 @@ private final PersistentTopics persistentTopics; private final NonPersistentTopics nonPersistentTopics; private final ResourceQuotas resourceQuotas; - + private final ClientConfigurationData clientConfigData; Review comment: nit: better use spaces rather than tab This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] merlimat closed pull request #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead
merlimat closed pull request #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead URL: https://github.com/apache/incubator-pulsar/pull/1365 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java index 617682169..08cd7f654 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java @@ -44,6 +44,7 @@ import org.apache.pulsar.client.api.PulsarClientException; import org.apache.pulsar.client.api.SubscriptionType; import org.apache.pulsar.client.impl.MessageIdImpl; +import org.apache.pulsar.client.impl.TopicMessageImpl; import org.apache.pulsar.common.api.proto.PulsarApi.CommandSubscribe.SubType; import org.apache.pulsar.common.naming.TopicName; import org.apache.pulsar.common.util.FutureUtil; @@ -323,13 +324,11 @@ public void testSimpleConsumerEventsWithPartition() throws Exception { final String subName = "sub1"; final int numMsgs = 100; Set uniqueMessages = new HashSet<>(); - admin.persistentTopics().createPartitionedTopic(topicName, numPartitions); ConsumerBuilderconsumerBuilder = pulsarClient.newConsumer().topic(topicName).subscriptionName(subName) .subscriptionType(SubscriptionType.Failover); - // 1. two consumers on the same subscription ActiveInactiveListenerEvent listener1 = new ActiveInactiveListenerEvent(); ActiveInactiveListenerEvent listener2 = new ActiveInactiveListenerEvent(); @@ -374,7 +373,7 @@ public void testSimpleConsumerEventsWithPartition() throws Exception { } totalMessages++; consumer1.acknowledge(msg); -MessageIdImpl msgId = (MessageIdImpl) msg.getMessageId(); +MessageIdImpl msgId = (MessageIdImpl) (((TopicMessageImpl)msg).getInnerMessageId()); receivedPtns.add(msgId.getPartitionIndex()); } @@ -391,7 +390,7 @@ public void testSimpleConsumerEventsWithPartition() throws Exception { } totalMessages++; consumer2.acknowledge(msg); -MessageIdImpl msgId = (MessageIdImpl) msg.getMessageId(); +MessageIdImpl msgId = (MessageIdImpl) (((TopicMessageImpl)msg).getInnerMessageId()); receivedPtns.add(msgId.getPartitionIndex()); } assertTrue(Sets.difference(listener1.inactivePtns, receivedPtns).isEmpty()); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java index 4a9912dce..6d6fc924c 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java @@ -208,7 +208,8 @@ public void partitionedProducerSendAsync() throws PulsarClientException, PulsarA Assert.assertEquals(messageIds.size(), numberOfMessages, "Not all messages published successfully"); for (int i = 0; i < numberOfMessages; i++) { -MessageId messageId = consumer.receive().getMessageId(); +MessageId topicMessageId = consumer.receive().getMessageId(); +MessageId messageId = ((TopicMessageIdImpl)topicMessageId).getInnerMessageId(); log.info("Message ID Received = " + messageId); Assert.assertTrue(messageIds.remove(messageId), "Failed to receive Message"); } @@ -247,7 +248,9 @@ public void partitionedProducerSend() throws PulsarClientException, PulsarAdminE Assert.assertEquals(messageIds.size(), numberOfMessages, "Not all messages published successfully"); for (int i = 0; i < numberOfMessages; i++) { - Assert.assertTrue(messageIds.remove(consumer.receive().getMessageId()), "Failed to receive Message"); +MessageId topicMessageId = consumer.receive().getMessageId(); +MessageId messageId = ((TopicMessageIdImpl)topicMessageId).getInnerMessageId(); +Assert.assertTrue(messageIds.remove(messageId), "Failed to receive Message"); } log.info("Message IDs = " + messageIds); Assert.assertEquals(messageIds.size(), 0, "Not all messages received successfully"); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/PatternTopicsConsumerImplTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/PatternTopicsConsumerImplTest.java index
[incubator-pulsar] branch master updated: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead (#1365)
This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pulsar.git The following commit(s) were added to refs/heads/master by this push: new 1dd9c43 Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead (#1365) 1dd9c43 is described below commit 1dd9c43e8e266d3957177354f592015c6f89c6d6 Author: Jia ZhaiAuthorDate: Tue Apr 3 22:03:55 2018 -0700 Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead (#1365) * delete partitionedConsumer, use topicsConsumer instead * change following comments * rebase master, rename TopicsConsumerImpl to MultiTopicsConsumerImpl * avoid dup calling getPartitionedTopicMetadata * rebase master, fix test error --- .../broker/service/PersistentFailoverE2ETest.java | 7 +- .../apache/pulsar/client/impl/MessageIdTest.java | 7 +- .../client/impl/PatternTopicsConsumerImplTest.java | 76 +-- .../PerMessageUnAcknowledgedRedeliveryTest.java| 6 +- .../pulsar/client/impl/TopicsConsumerImplTest.java | 46 +- .../apache/pulsar/client/impl/ConsumerImpl.java| 2 +- ...sumerImpl.java => MultiTopicsConsumerImpl.java} | 283 ++- .../client/impl/PartitionedConsumerImpl.java | 553 - ...pl.java => PatternMultiTopicsConsumerImpl.java} | 24 +- .../pulsar/client/impl/PulsarClientImpl.java | 8 +- .../pulsar/client/impl/TopicMessageImpl.java | 6 +- 11 files changed, 263 insertions(+), 755 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java index 6176821..08cd7f6 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentFailoverE2ETest.java @@ -44,6 +44,7 @@ import org.apache.pulsar.client.api.Producer; import org.apache.pulsar.client.api.PulsarClientException; import org.apache.pulsar.client.api.SubscriptionType; import org.apache.pulsar.client.impl.MessageIdImpl; +import org.apache.pulsar.client.impl.TopicMessageImpl; import org.apache.pulsar.common.api.proto.PulsarApi.CommandSubscribe.SubType; import org.apache.pulsar.common.naming.TopicName; import org.apache.pulsar.common.util.FutureUtil; @@ -323,13 +324,11 @@ public class PersistentFailoverE2ETest extends BrokerTestBase { final String subName = "sub1"; final int numMsgs = 100; Set uniqueMessages = new HashSet<>(); - admin.persistentTopics().createPartitionedTopic(topicName, numPartitions); ConsumerBuilder consumerBuilder = pulsarClient.newConsumer().topic(topicName).subscriptionName(subName) .subscriptionType(SubscriptionType.Failover); - // 1. two consumers on the same subscription ActiveInactiveListenerEvent listener1 = new ActiveInactiveListenerEvent(); ActiveInactiveListenerEvent listener2 = new ActiveInactiveListenerEvent(); @@ -374,7 +373,7 @@ public class PersistentFailoverE2ETest extends BrokerTestBase { } totalMessages++; consumer1.acknowledge(msg); -MessageIdImpl msgId = (MessageIdImpl) msg.getMessageId(); +MessageIdImpl msgId = (MessageIdImpl) (((TopicMessageImpl)msg).getInnerMessageId()); receivedPtns.add(msgId.getPartitionIndex()); } @@ -391,7 +390,7 @@ public class PersistentFailoverE2ETest extends BrokerTestBase { } totalMessages++; consumer2.acknowledge(msg); -MessageIdImpl msgId = (MessageIdImpl) msg.getMessageId(); +MessageIdImpl msgId = (MessageIdImpl) (((TopicMessageImpl)msg).getInnerMessageId()); receivedPtns.add(msgId.getPartitionIndex()); } assertTrue(Sets.difference(listener1.inactivePtns, receivedPtns).isEmpty()); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java index 4a9912d..6d6fc92 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageIdTest.java @@ -208,7 +208,8 @@ public class MessageIdTest extends BrokerTestBase { Assert.assertEquals(messageIds.size(), numberOfMessages, "Not all messages published successfully"); for (int i = 0; i < numberOfMessages; i++) { -MessageId messageId = consumer.receive().getMessageId(); +MessageId topicMessageId = consumer.receive().getMessageId(); +MessageId messageId = ((TopicMessageIdImpl)topicMessageId).getInnerMessageId();
[GitHub] merlimat commented on a change in pull request #1494: In Namespace.addClusters and Properties.addRoles changed dataType to Set
merlimat commented on a change in pull request #1494: In Namespace.addClusters and Properties.addRoles changed dataType to Set URL: https://github.com/apache/incubator-pulsar/pull/1494#discussion_r179027633 ## File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java ## @@ -388,7 +388,7 @@ protected void internalSetNamespaceReplicationClusters(List clusterIds) // Force to read the data s.t. the watch to the cache content is setup. policiesNode = policiesCache().getWithStat(path(POLICIES, namespaceName.toString())).orElseThrow( () -> new RestException(Status.NOT_FOUND, "Namespace " + namespaceName + " does not exist")); -policiesNode.getKey().replication_clusters = clusterIds; +policiesNode.getKey().replication_clusters = Lists.newArrayList(replicationClusterSet); Review comment: Can we also make `replication_clusters` as a set? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sijie commented on issue #1458: skip-all/clear-backlog doesn't work if bookkeeper is outage
sijie commented on issue #1458: skip-all/clear-backlog doesn't work if bookkeeper is outage URL: https://github.com/apache/incubator-pulsar/issues/1458#issuecomment-378453193 @merlimat it seems #1461 doesn't actually address the problem. I am seeing 404. it seems to me that there is no topic reference - https://github.com/apache/incubator-pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L642 so it doesn't actually touch the skip-all logic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zhaijack opened a new pull request #1495: Add MultiTopicsConsumerImpl with leaked commit from partitionedConsumer
zhaijack opened a new pull request #1495: Add MultiTopicsConsumerImpl with leaked commit from partitionedConsumer URL: https://github.com/apache/incubator-pulsar/pull/1495 ### Motivation MultiTopicsConsumerImpl leaked merge the change in partitionedConsumer in [PR1462] (https://github.com/apache/incubator-pulsar/pull/1462/files#diff-bbced2cbb73414dfcac0c55dc9169d37R427). It will cause unit-test `AdminApiTest.partitionedTopicsCursorReset` fail some times, when there is a delay of `AcknowledgmentsGroupingTracker.flush` ### Modifications add leaked changes back. ### Result should pass all unit-test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services