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.

Sign up for GitHub

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

Jump to bottom

DbLedgerStorage does not persist ExplicitLAC: Information (Header) of TransientLedgerInfo is not persisted #1533

Open
reddycharan opened this issue Jul 3, 2018 · 3 comments
Open

DbLedgerStorage does not persist ExplicitLAC: Information (Header) of TransientLedgerInfo is not persisted #1533

reddycharan opened this issue Jul 3, 2018 · 3 comments
Assignees
@eolivelli
Labels
area/bookie triage/week-8 type/bug

Comments

@reddycharan
Copy link
Contributor

reddycharan commented Jul 3, 2018 •
edited by eolivelli
Loading

FEATURE REQUEST

  1. Please describe the feature you are requesting.

TransientLedgerInfo class borrows the logic from FileInfo, which is used for holding all the transient states for a given ledger when DbLedgerStorage is used for LedgerStorage. But unlike FileInfo, in TransientLedgerInfo, Header (containing masterKey, stateBits and explicitLac) is not defined and it is not persisted.

  1. Indicate the importance of this issue to you (blocker, must-have, should-have, nice-to-have). Are you currently using any workarounds to address this issue?

With #1527 we are going to persist explicitLac, but since TransientLedgerInfo doesn't has Header/persistence logic, for now I'm going to ignore persisting explicitLac logic in TransientLedgerInfo/DbLedgerStorage case.

Check Ignored test case here:
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ExplicitLacTest.java#L145
@reddycharan
Copy link
Contributor Author

Hey @sijie , as we discussed earlier, created open issue for Header/persistence logic in TransientLedgerInfo

@merlimat FYI

@eolivelli
Copy link
Contributor

eolivelli commented Jul 3, 2018 •
edited
Loading

This means that with DbLedgerStorage we will not persist ExplicitLAC ?
Are you planning to implement it in a follow up patch? Otherwise we should take care of documenting this important difference in DbLedgerStorage vs SortedLedgerStorage

@reddycharan reddycharan mentioned this issue Jul 25, 2018
Closed
@sijie sijie mentioned this issue Jan 15, 2020
Open
@eolivelli eolivelli mentioned this issue Oct 27, 2020
Merged
@eolivelli eolivelli changed the title Information (Header) of TransientLedgerInfo is not persisted DbLedgerStorage does not persist ExplicitLAC: Information (Header) of TransientLedgerInfo is not persisted Oct 29, 2020
@eolivelli
Copy link
Contributor

I am working on this task.
Hopefully we can deliver this fix for 4.12.0.

@eolivelli eolivelli self-assigned this Oct 31, 2020
@eolivelli eolivelli mentioned this issue Oct 31, 2020
Merged
jiazhai pushed a commit that referenced this issue Nov 2, 2020
@eolivelli
Issue 1533: DbLedgerStorage does not persist ExplicitLAC: Information…
4d44650 
… (Header) of TransientLedgerInfo is not persisted (#2474)

Motivation
DbLedgerStorage does not persist "ExplicitLAC" on RocksDB, so when you restart a Bookie the information is lost.
This work can be considered a follow up of @reddycharan work at #1527

Changes
persist ExplicitLAC on DBLedgerStorage
save ExplicitLAC on RocksDB (it is an optional field, so this change is backward compatible)
enable testExplicitLACIsPersisted test even for DBLedgerStorage
on TransientLedgerInfo rewind the ByteBuf in order to be able to use it again while writing to RocksDB
use computeIfAbsent in getOrAddLedgerInfo (code clean up, but code looked strange, probably subject to some possible race condition)
Master Issue: #1533


* Issue 1533: DbLedgerStorage does not persist ExplicitLAC: Information (Header) of TransientLedgerInfo is not persisted

* final fix and fix checkstyle

* restore tests

Co-authored-by: Enrico Olivelli <eolivelli@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eolivelli eolivelli

Labels
area/bookie triage/week-8 type/bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sijie @eolivelli @reddycharan