Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BOOKKEEPER-1088: Ledger Recovery (part-3) - Add a ReadEntryListener to callback on individual request #178

Closed
wants to merge 7 commits into from

Conversation

sijie
Copy link
Member

@sijie sijie commented Jun 1, 2017

THIS CHANGE IS BASED ON #177 (you can review 868a3c8 for the only change that belongs to BOOKKEEPER-1088).

bookkeeper recovery improvement (part-3): add a ReadEntryListener to callback on individual request.

  • add read entry listener which allow doing batch read, but callback on individual entries in sequence. so in recovery op, we could issue batch reads, then on each individual callback do add entry and stop when received NoSuchEntry.

Sijie Guo and others added 5 commits June 1, 2017 11:48
this change is the first part of improving bookkeeper recovery. it is basically a refactor change, which:

- abstract an interface for LedgerEntryRequest in PendingReadOp
- rename current implementation to SequenceReadRequest, which read the entry in the sequence of quorum.

RB_ID=266137
…est in PendingReadOp

- add a parallel reading request in PendingReadOp
- allow PendingReadOp to configure whether to do parallel reading or not
- add flag in ClientConfiguration to allow configuring whether to do parallel reading in LedgerRecoveryOp or not.

RB_ID=266139
…callback on individual request.

- add read entry listener which allow doing batch read, but callback on individual entries in sequence. so in recovery op, we could issue batch reads, then on each individual callback do add entry and stop when received NoSuchEntry.

RB_ID=266143
Copy link
Member

@jiazhai jiazhai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. LGTM

@jvrao
Copy link
Contributor

jvrao commented Jun 2, 2017

LGTM

@eolivelli
Copy link
Contributor

LGTM +1

@sijie sijie changed the title BOOKKEEPER-1088: Ledger Recovery - Add a ReadEntryListener to callback on individual request BOOKKEEPER-1088: Ledger Recovery (part-3) - Add a ReadEntryListener to callback on individual request Jun 2, 2017
@asfgit asfgit closed this in 5fe8652 Jun 5, 2017
asfgit pushed a commit that referenced this pull request Jun 21, 2017
…er recovery

This change is based on #178 - (you can review git sha 82f73ef)

bookkeeper recovery improvement (part-4): allow batch reading in ledger recovery

    - enable batch read in ledger recovery, so we could parallel reading to improve recovery time.

    RB_ID=266145

Author: Sijie Guo <sijieg@twitter.com>
Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Matteo Merli <mmerli@apache.org>

This closes #181 from sijie/recovery_improvements_part4
asfgit pushed a commit that referenced this pull request Jun 21, 2017
This change is based #178 - (you can review git sha 40ca8c2)

bookkeeper: LAC piggyback at read response

        - bookie server changes
          * cache maximum lac in file info
          * provide getLastAddConfirmed & setLastAddConfirmed in ledger storage
          * addEntry will set its lac thru setLastAddConfirmed
          * readEntry will read latest lac and piggyback
        - client change
          * check whether the response has lac piggybacked, if there is lac update lac in its corresponding ledger handle.

Author: Sijie Guo <sijie@apache.org>
Author: Sijie Guo <sijieg@twitter.com>

Reviewers: Jia Zhai <None>, Matteo Merli <mmerli@apache.org>

This closes #180 from sijie/piggyback_lac
revans2 pushed a commit to revans2/bookkeeper that referenced this pull request Oct 11, 2017
YBK-3: Re-enabling skipped tests
@sijie sijie deleted the recovery_improvements_part3 branch July 16, 2018 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants