[
https://issues.apache.org/jira/browse/BOOKKEEPER-331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13414645#comment-13414645
]
Sijie Guo commented on BOOKKEEPER-331:
--------------------------------------
thanks Mridul for updates. the patches (BOOKKEEPER-309, BOOKKEEPER-310,
BOOKKEEPER-311) are good for me except some minor problems.
1) all patches need to rebase to latest trunk. a) applying
hedwig-protocol.patch.3 failed on PubSubProtocol.java; b) applying
hedwig-server.patch.3. it failed to compile test cases due to you don't change
TestCallback to return message id in TestBookKeeperPersistenceManager (which is
a new test case added in BOOKKEEPER-302).
2) need a test case to test returning message seq id. you could leverage
TestPubSubClient in hedwig-server module. those test cases could be publish &
asyncPublish several messages and checking the messages id returned by publish
and asyncPublish equals to the message seq id received by a subscriber.
3) hedwig-protocol.patch : why not use the patch in BOOKKEEPER-78 instead of
combining it together? so it could make each jira focus on its changes.
4) hedwig-client patch :
{code}
+ public void handleChannelClosedExplicitly(){
+ // Handle consume buffering, etc here - in a different patch
+ channelClosedExplicitly = true;
+ }
+
{code}
it would be better to add 'TODO' on the above comment.
{code}
+
+ PubSubProtocol.ResponseBody respBody =
pubSubCallback.getResponseBody();
+ if (null == respBody) return null;
+ return respBody.getPublishResponse();
{code}
since PublishReponse is an optional filed in ResponseBody, do we need to check
whether ResponseBody#hasPublishResponse() before #getPublishResponse() ?
BTW, you could put it together on this jira, so you could add test cases to run
for this feature. why I created this jira is because from the titles of
BOOKKEEPER-309, BOOKKEEPER-310, BOOKKEEPER-311, user could not know what it is
for. but from title of BOOKKEEPER-331, he could know it is used to support
returning message seq id. I think it is OK to mark BOOKKEEPER-309,
BOOKKEEPER-310, BOOKKEEPER-311 as duplicated of BOOKKEEPER-331. because there
is relationship between them and we don't lose any discussion.
> Let hedwig support returning message seq id for publish requests.
> -----------------------------------------------------------------
>
> Key: BOOKKEEPER-331
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-331
> Project: Bookkeeper
> Issue Type: Sub-task
> Components: hedwig-client, hedwig-server
> Reporter: Sijie Guo
> Fix For: 4.2.0
>
>
> Let hedwig support returning message seq id for published messages.
--
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
