[GitHub] rdhabalia commented on issue #1462: Delayed acks impl

2018-07-09 Thread GitBox
rdhabalia commented on issue #1462: Delayed acks impl
URL: https://github.com/apache/incubator-pulsar/pull/1462#issuecomment-403660668
 
 
   > The code should already be sending 1 single ack if the broker is < 2.0 :
   
   I see, I missed it to check.
   
   I think the issue is that I have cherry-picked 
[ActiveConsumerListener-change](https://github.com/apache/incubator-pulsar/pull/1156)
 into broker-1.22 to support function on broker-1.22 and this PR upgraded 
pulsar-broker version=`V12` and because of that client was sending group-ack to 
broker which broker couldn't understand.
   Thanks for pointing it out, I will fix internal broker's patch.


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] rdhabalia commented on issue #1462: Delayed acks impl

2018-07-09 Thread GitBox
rdhabalia commented on issue #1462: Delayed acks impl
URL: https://github.com/apache/incubator-pulsar/pull/1462#issuecomment-403400100
 
 
   @merlimat any update on 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] rdhabalia commented on issue #1462: Delayed acks impl

2018-07-07 Thread GitBox
rdhabalia commented on issue #1462: Delayed acks impl
URL: https://github.com/apache/incubator-pulsar/pull/1462#issuecomment-403257185
 
 
   can we add  `CommandAckDeprecated` in proto-schema which can be used by 
client if broker-verison is < `V12` ?


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] rdhabalia commented on issue #1462: Delayed acks impl

2018-07-07 Thread GitBox
rdhabalia commented on issue #1462: Delayed acks impl
URL: https://github.com/apache/incubator-pulsar/pull/1462#issuecomment-403256971
 
 
   @merlimat this is not a backward compatible change with old broker (version 
< 2.0) because, old brokers don't understand group-ack command so, combination 
of new-client (>=2.0) and old-broker will not work and it can create ack-holes 
for the client.
   
   also `CommandAck` schema is also changed 
   ```
   Before: required MessageIdData message_id = 3;
   After: repeated MessageIdData message_id = 3;
   ```
   
   I think we were making sure that we don't break backward compatibility so, 
If client uses new version and if it requires to rollback broker version then 
it can not create an issue here. Any thoughts to fix this issue?


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