[
https://issues.apache.org/jira/browse/BOOKKEEPER-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13532984#comment-13532984
]
Flavio Junqueira commented on BOOKKEEPER-509:
---------------------------------------------
The patch looks mostly good, I just have a few quick comments:
# The obvious one is the trailing spaces warning in the jenkins report.
# The first LOG.info in the patch should be LOG.debug instead.
# Really minor, but I don't really like the name of the variable
"isNewCompletedRequest". Something like "isComplete" or "entryComplete" might
be better, and add a comment to say that it is true for any given entry at most
once. It might also be a good idea to add some comment to the complete() call
to explain that it returns false in the case the read entry request is not
complete and in the case it is complete but has been checked before. Although
it is clear from the code, I find that it is good practice to also explain in
words so that we can later verify that the code and the explanation match.
Again, these are just suggestions based on my preferences.
> TestBookKeeperPersistenceManager failed on latest trunk
> -------------------------------------------------------
>
> Key: BOOKKEEPER-509
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-509
> Project: Bookkeeper
> Issue Type: Bug
> Components: hedwig-server
> Reporter: Sijie Guo
> Assignee: Sijie Guo
> Priority: Blocker
> Fix For: 4.2.0
>
> Attachments: BOOKKEEPER-509.diff, BOOKKEEPER-509.diff,
> hedwig-server.log, hedwig-server.log.bak
>
>
> latest trunk failed at TestBookKeeperPersistenceManager.
> sees that it caused by uncaught exception:
> {code}
> java.util.NoSuchElementException
> at java.util.AbstractQueue.remove(AbstractQueue.java:90)
> at
> org.apache.bookkeeper.client.PendingReadOp.nextElement(PendingReadOp.java:345)
> at
> org.apache.bookkeeper.client.PendingReadOp.nextElement(PendingReadOp.java:53)
> at
> org.apache.bookkeeper.client.LedgerRecoveryOp.readComplete(LedgerRecoveryOp.java:100)
> at
> org.apache.bookkeeper.client.PendingReadOp.submitCallback(PendingReadOp.java:338)
> at
> org.apache.bookkeeper.client.PendingReadOp.readEntryComplete(PendingReadOp.java:327)
> at
> org.apache.bookkeeper.proto.PerChannelBookieClient.handleReadResponse(PerChannelBookieClient.java:627)
> at
> org.apache.bookkeeper.proto.PerChannelBookieClient$7.safeRun(PerChannelBookieClient.java:529)
> at org.apache.bookkeeper.util.SafeRunnable.run(SafeRunnable.java:31)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:439)
> at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
> at java.util.concurrent.FutureTask.run(FutureTask.java:138)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
> at java.lang.Thread.run(Thread.java:680)
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
