[ 
https://issues.apache.org/jira/browse/BOOKKEEPER-867?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14733882#comment-14733882
 ] 

Sijie Guo commented on BOOKKEEPER-867:
--------------------------------------

A couple comments:

- It might not worth changing the ConcurrentLinkedQueue for LedgerHandle. it 
would impact all people that uses LedgerHandle. there isn't performance side 
effects using PriorityBlockingQueue. You could use PriorityBlockingQueue in 
WriteLedgerHandle.
- I'd suggest not adding addEntry(long EntryId ..) methods to LedgerHandle.
  * it might be worth to call WriteLedgerHandle as LedgerHandleAdv.
  * introduce a new CreateAdvCallback, which only returns  LedgerHandleAdv.
  * move most of LedgerHandle's code (except addEntry) to AbstractLedgerHandle, 
and let LedgerHandle extend AbstractLedgerHandle and provide addEntry methods 
(without entry id). and let LedgerHandleAdv extend AbstractLedgerHandle and 
provide addEntry method with entry id. so there won't be two set of addEntry 
apis in each ledger handle class.
- addEntry with entry ids should have the logic to prevent adding duplicated 
entries.
- tests should cover add entries but out-of-order entry ids (not just reverse 
order) and might be test cases cover gap.

It would be good to attach this patch to review board : 
https://reviews.apache.org/dashboard/ It is a patch of new API, it would be 
easier to comment on review board.

> New Client API to allow applications pass-in EntryId.
> -----------------------------------------------------
>
>                 Key: BOOKKEEPER-867
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-867
>             Project: Bookkeeper
>          Issue Type: Sub-task
>          Components: bookkeeper-client
>            Reporter: Venkateswararao Jujjuri
>            Assignee: Venkateswararao Jujjuri
>              Labels: features, newbie
>             Fix For: 4.4.0
>
>         Attachments: 
> 0001-BOOKKEEPER-867-New-Client-API-to-allow-applications-.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to