[
https://issues.apache.org/jira/browse/BOOKKEEPER-311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409319#comment-13409319
]
Mridul Muralidharan commented on BOOKKEEPER-311:
------------------------------------------------
bq. 1) the changes break backward compatibility. it changed the consume
semantic. if existed applications used consume as async callback, it broken
existed applications' assumption. one way I think we might not change consume's
semantic, adding another call which returns only when consume request is
written to the channel. this new call name might be 'syncConsume' or other
better name.
When discussing with Ivan, it looked like the existing consume() method was not
consistent with rest of the api exposed from the interface all async methods
were prefixed with 'async' and the sync ones were not.
We assumed the implementation of consume was an oversight (particularly since
it did not have a async variant) - hence the addition of an additional explicit
asyncConsume.
Having said that, you are right, it is an intentional interface behavioral
change introduced.
Note that there are two aspects to the overall change :
Basically, what happens when user does
* consume() and then close() (no-auto ack mode) vs
* close() (ack mode - controlled via auto_send_consume_message_enabled=true
(default) and consumed_messages_buffer_size (default 5) ).
This in context of the fact that there is option of automatic consume
acknowledgement, and buffering of these consume acknowledgements implemented in
java api. (Actually, you can mix ack-mode with explicit consume() too, and vice
versa btw - the latter resulting in some interesting issues).
The changes to client code (on consume related changes) are to handle issues
with these two cases.
In the first case :
If consume remains async, there are two possibilities here :
1) consume() results in request being sent to server, and so server does not
redeliver the message again.
2) consume() request was in flight (within netty), while close() is executed -
resulting in socket close before request makes it out.
As you mentioned, we can ofcourse keep consume() as async (but with no way to
track progress via a future) and introduce a syncConsume() - functionally,
other than inability to track future, this will be functionally equivalent way
to resolve the bug !
Note that, as you mentioned in point 2 above, other than netty 'telling us'
about delivery of request to server - there is nothing much else we can do.
Practically, this is good enough.
It is a best case effort, and does not give transactional gaurantee's.
For the second case (close() with auto-ack) :
The changes to *ResponseHandler via addition of handleChannelClosedExplicitly
(rename ?) is to handle this case.
Ensure that buffered state (in this case, consume'd seq-id which is not yet
ack'ed to server) is written before closing socket.
Even this is, ofcourse, best case effort : note that this is used for sending
buffered seq-id but could be used for others too (now or in future).
(1) is the desired behavior, and (2) is actually fairly common - please note
that this observed and then fixed, not other way around :-) I was really
looking to no change hedwig in any way possible.
a) The sync aspect comes in ONLY when the invocation of the method results in a
request being sent to the server.
b) If consume() does not result in request to server (due to buffering of
consume requests), then changes to close() handle the delivery of buffered
seq'id.
To recap:
The issue we are trying to resolve is, if message is consume'd by user, within
reasonable gaurantee's, it must not be sent back to him.
Problem (2) above meant that upto last 4 messages might always be sent back to
client.
Problem (1) meant that consume + close might send last message (or last N if
batch consume by user !) back to him.
> Changes in hedwig client api to support JMS spec
> ------------------------------------------------
>
> Key: BOOKKEEPER-311
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-311
> Project: Bookkeeper
> Issue Type: Sub-task
> Reporter: Mridul Muralidharan
> Attachments: hedwig-client-consume.patch.1,
> hedwig-client-publish.patch.1, hedwig-client.patch
>
>
> Primary changes are :
> a) Add support for returning seq-id for a publish request. This is an api
> change (backwardly compatible for users).
> b) Make consume a sync consume, with addition of an asyncConsume - this is to
> ensure that invoking consume() ensure request makes to server before
> returning (with what reasonable gaurantees that netty allows).
> c) Ensure that explicit close'ing of session will flush buffered consume
> seq-id's when auto-ack is enabled (default in hedwig java client).
> In addition, there are also fixes for
> d) Fix NPE's observed as part of testing JMS provider.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
