[GitHub] zhaijack commented on issue #1365: Delete PartitionedConsumerImpl, use TopicsConsumerImpl instead

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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)

2018-04-03 Thread mmerli
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 Merli 
AuthorDate: 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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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)

2018-04-03 Thread sijie
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 Merli 
AuthorDate: 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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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)

2018-04-03 Thread GitBox
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)

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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 Map selectedBundlesCache;
 
-/**
- * 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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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);
 
 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 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)

2018-04-03 Thread mmerli
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 Zhai 
AuthorDate: 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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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

2018-04-03 Thread GitBox
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