You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Sijie Guo (JIRA)" <ji...@apache.org> on 2015/09/07 18:19:45 UTC
[jira] [Commented] (BOOKKEEPER-867) New Client API to allow
applications pass-in EntryId.
[ 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)
Re: [jira] [Commented] (BOOKKEEPER-867) New Client API to allow
applications pass-in EntryId.
Posted by Sijie Guo <si...@apache.org>.
On Mon, Sep 7, 2015 at 10:24 PM, Venkateswara Rao Jujjuri <jujjuri@gmail.com
> wrote:
> Can you explain little more on "might be test cases cover gap."?
>
for example, client adds 1,2,3 and 5. 5 will never succeed. and make sure
ledger close and ledger recovery work as expected when there is a gap.
>
> You mean ledger getting closed with gaps? or something else?
>
> Regarding your suggestion of creating abstract class, you basically wanted
> AdvLedgerHandle users to deal two handles,
> for reading(trailing or general) use LedgerHandle(ReadOnlyLedgerHandle)
> and for writing, use AdvLedgerHandle.
>
correct.
>
> Thanks,
> JV
>
> On Mon, Sep 7, 2015 at 11:15 AM, Venkateswara Rao Jujjuri <
> jujjuri@gmail.com
> > wrote:
>
> > Thanks Sijie.
> >
> > On Mon, Sep 7, 2015 at 9:19 AM, Sijie Guo (JIRA) <ji...@apache.org>
> wrote:
> >
> >>
> >> [
> >>
> 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)
> >>
> >
> >
> >
> > --
> > Jvrao
> > ---
> > First they ignore you, then they laugh at you, then they fight you, then
> > you win. - Mahatma Gandhi
> >
> >
> >
>
>
> --
> Jvrao
> ---
> First they ignore you, then they laugh at you, then they fight you, then
> you win. - Mahatma Gandhi
>
Re: [jira] [Commented] (BOOKKEEPER-867) New Client API to allow
applications pass-in EntryId.
Posted by Venkateswara Rao Jujjuri <ju...@gmail.com>.
Can you explain little more on "might be test cases cover gap."?
You mean ledger getting closed with gaps? or something else?
Regarding your suggestion of creating abstract class, you basically wanted
AdvLedgerHandle users to deal two handles,
for reading(trailing or general) use LedgerHandle(ReadOnlyLedgerHandle)
and for writing, use AdvLedgerHandle.
Thanks,
JV
On Mon, Sep 7, 2015 at 11:15 AM, Venkateswara Rao Jujjuri <jujjuri@gmail.com
> wrote:
> Thanks Sijie.
>
> On Mon, Sep 7, 2015 at 9:19 AM, Sijie Guo (JIRA) <ji...@apache.org> wrote:
>
>>
>> [
>> 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)
>>
>
>
>
> --
> Jvrao
> ---
> First they ignore you, then they laugh at you, then they fight you, then
> you win. - Mahatma Gandhi
>
>
>
--
Jvrao
---
First they ignore you, then they laugh at you, then they fight you, then
you win. - Mahatma Gandhi
Re: [jira] [Commented] (BOOKKEEPER-867) New Client API to allow
applications pass-in EntryId.
Posted by Venkateswara Rao Jujjuri <ju...@gmail.com>.
Thanks Sijie.
On Mon, Sep 7, 2015 at 9:19 AM, Sijie Guo (JIRA) <ji...@apache.org> wrote:
>
> [
> 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)
>
--
Jvrao
---
First they ignore you, then they laugh at you, then they fight you, then
you win. - Mahatma Gandhi