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
Issue #2908: Replace unsafe NoEntryException with IOException #2909
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ping @Vanlightly, Would you please rebase the master? thanks. |
ping @merlimat @eolivelli, Would you please help take a look? thanks a lot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
2023e56
to
f5b01bb
Compare
I just want to merge master branch into this, found that introduces some unnecessary commits. So I revert it. |
Rebase is easier than merge master, but that needs to force push the first commit. If @Vanlightly won't have time to do that, I would like to help with this PR in the next monday. |
Throwing a NoEntryException from the entry logger for an entry that the index says should exist is unsafe. It can cause ledger truncation during ledger recovery. It only takes a single false NoSuchEntry response to trigger truncation. NoEntryException should only be thrown from inside ledger storage if the entry is not found in the index.
f5b01bb
to
af3cbff
Compare
* Replace unsafe NoEntryException with IOException Throwing a NoEntryException from the entry logger for an entry that the index says should exist is unsafe. It can cause ledger truncation during ledger recovery. It only takes a single false NoSuchEntry response to trigger truncation. NoEntryException should only be thrown from inside ledger storage if the entry is not found in the index. * fix CI Co-authored-by: chenhang <chenhang@apache.org> (cherry picked from commit 966b865)
…pache#2909) * Replace unsafe NoEntryException with IOException Throwing a NoEntryException from the entry logger for an entry that the index says should exist is unsafe. It can cause ledger truncation during ledger recovery. It only takes a single false NoSuchEntry response to trigger truncation. NoEntryException should only be thrown from inside ledger storage if the entry is not found in the index. * fix CI Co-authored-by: chenhang <chenhang@apache.org> (cherry picked from commit 966b865)
…pache#2909) * Replace unsafe NoEntryException with IOException Throwing a NoEntryException from the entry logger for an entry that the index says should exist is unsafe. It can cause ledger truncation during ledger recovery. It only takes a single false NoSuchEntry response to trigger truncation. NoEntryException should only be thrown from inside ledger storage if the entry is not found in the index. * fix CI Co-authored-by: chenhang <chenhang@apache.org> (cherry picked from commit 966b865)
…pache#2909) * Replace unsafe NoEntryException with IOException Throwing a NoEntryException from the entry logger for an entry that the index says should exist is unsafe. It can cause ledger truncation during ledger recovery. It only takes a single false NoSuchEntry response to trigger truncation. NoEntryException should only be thrown from inside ledger storage if the entry is not found in the index. * fix CI Co-authored-by: chenhang <chenhang@apache.org> (cherry picked from commit 966b865) (cherry picked from commit 23355e0)
Motivation
Throwing a NoEntryException from the entry logger
for an entry that the index says should exist is
unsafe. It can cause ledger truncation during ledger
recovery. It only takes a single false NoSuchEntry
response to trigger truncation.
NoEntryException should only be thrown from inside
ledger storage if the entry is not found in the index.
Changes
Do not throw NoEntryException from EntryLogger on failing to read entry from log file, throw IOException instead.
Master Issue: #2908