[GitHub] jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries
jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries URL: https://github.com/apache/bookkeeper/pull/727#issuecomment-346598103 seems, it is not able to merge with merge script, it reported: ``` bookkeeper git:(master) ? python dev/bk-merge-pr.py git rev-parse --abbrev-ref HEAD Which pull request would you like to merge? (e.g. 34): 727 No approvals found in this pull request git rev-parse --abbrev-ref HEAD git branch ``` the below message is to test again: +1, lgtm. 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] jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries
jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries URL: https://github.com/apache/bookkeeper/pull/727#issuecomment-346308299 @ivankelly @sijie 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] jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries
jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries URL: https://github.com/apache/bookkeeper/pull/727#issuecomment-346301408 @ivan, Thanks for the comments, the reason is described in the issue, will move it 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] jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries
jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries URL: https://github.com/apache/bookkeeper/pull/727#issuecomment-346249325 Rebased master, since the LedgerEntry changes. @sijie @eolivelli Would you please help check this again. 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] jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries
jiazhai commented on issue #727: Issue 693: add interface and implementation of LedgerEntries URL: https://github.com/apache/bookkeeper/pull/727#issuecomment-344606742 @eolivelli, Regarding the release/retain story, Here is the understanding: Before this change, read() return result as Iterable, and if user call Iterable.iterator() multiple times, the references of ByteBuf have potential to be broken, and hard to manage. e.g. one of the iterator accessed some entry and then released the reference; while another iterator may still want to access the same entry. By this change, each call of iterator() will inc the references, and after usage, caller will dec the reference, it is manage-able and more safe. 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
