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

EntryMemTable.newEntry retains reference to passed ByteBuffer array, can cause corruption on journal replay #1737

Closed
athanatos opened this issue Oct 4, 2018 · 0 comments
Closed

EntryMemTable.newEntry retains reference to passed ByteBuffer array, can cause corruption on journal replay #1737

athanatos opened this issue Oct 4, 2018 · 0 comments
Assignees
@athanatos
Labels
area/bookie durability priority/important release/4.7.3 release/4.8.1 release/4.9.0

Comments

@athanatos
Copy link

athanatos commented Oct 4, 2018 •
edited
Loading

Journal.scanJournal reuses the same ByteBuffer for each entry, but (if the entry is large enough) EntryMemTable retains a reference to that array for each affected entry. The result is that if any entry is large enough, the bytes that get written to the entryLog upon flush for that entry will be a prefix of the last entry replayed from that journal file. This will in general corrupt any such entries. Fix seems relatively simple, PR will follow.
@athanatos athanatos self-assigned this Oct 4, 2018
athanatos pushed a commit to athanatos/bookkeeper that referenced this issue Oct 5, 2018
ISSUE apache#1737: EntryMemTable.newEntry: always make a copy
b9cb772 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(@bug W-5499346@)
(@Rev cguttapalem@)
Signed-off-by: Samuel Just <sjust@salesforce.com>
athanatos pushed a commit to athanatos/bookkeeper that referenced this issue Oct 8, 2018
ISSUE apache#1737: EntryMemTable.newEntry: always make a copy
77d6899 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(bug W-5499346)
(rev cguttapalem)
Signed-off-by: Samuel Just <sjustsalesforce.com>

Author: 

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Matteo Merli <mmerli@apache.org>, Sijie Guo <sijie@apache.org>

This closes apache#1744 from athanatos/forupstream/wip-1737, closes apache#1737

(cherry picked from commit df65958)
athanatos pushed a commit to athanatos/bookkeeper that referenced this issue Oct 8, 2018
ISSUE apache#1737: EntryMemTable.newEntry: always make a copy
b75a0a3 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(bug W-5499346)
(rev cguttapalem)
Signed-off-by: Samuel Just <sjustsalesforce.com>

Author: 

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Matteo Merli <mmerli@apache.org>, Sijie Guo <sijie@apache.org>

This closes apache#1744 from athanatos/forupstream/wip-1737, closes apache#1737

(cherry picked from commit df65958)
This was referenced Oct 8, 2018
Merged
Merged
sijie pushed a commit that referenced this issue Oct 9, 2018
@sijie
ISSUE #1737: EntryMemTable.newEntry: always make a copy
675de3e 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(bug W-5499346)
(rev cguttapalem)
Signed-off-by: Samuel Just <sjustsalesforce.com>

Author: 

Reviewers: Enrico Olivelli <eolivelligmail.com>, Matteo Merli <mmerliapache.org>, Sijie Guo <sijieapache.org>

This closes #1744 from athanatos/forupstream/wip-1737, closes #1737

(cherry picked from commit df65958)

Author: Enrico Olivelli <eolivelli@apache.org>
Author: Matteo Merli <mmerli@apache.org>
Author: Sijie Guo <sijie@apache.org>
Author: cguttapalem <cguttapalem@salesforce.com>
Author: Qi Wang <codingwangqi@gmail.com>
Author: Ivan Kelly <ivank@apache.org>
Author: JV Jujjuri <vjujjuri@salesforce.com>
Author: Andrey Yegorov <ayegorov@salesforce.com>
Author: Samuel Just <sjust@salesforce.com>
Author: Enrico Olivelli <eolivelli@gmail.com>
Author: Khurrum Nasimm <khurrumnasimm@gmail.com>
Author: Charan Reddy Guttapalem <reddycharan18@gmail.com>
Author: Sijie Guo <guosijie@gmail.com>
Author: Andrey Yegorov <dlg99@users.noreply.github.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1745 from athanatos/forupstream/wip-1744-4.8, closes #1737
sijie pushed a commit that referenced this issue Oct 9, 2018
@sijie
ISSUE #1737: EntryMemTable.newEntry: always make a copy
fef7f00 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(bug W-5499346)
(rev cguttapalem)
Signed-off-by: Samuel Just <sjustsalesforce.com>

Author: 

Reviewers: Enrico Olivelli <eolivelligmail.com>, Matteo Merli <mmerliapache.org>, Sijie Guo <sijieapache.org>

This closes #1744 from athanatos/forupstream/wip-1737, closes #1737

(cherry picked from commit df65958)

Author: Sijie Guo <sijie@apache.org>
Author: Enrico Olivelli <eolivelli@apache.org>
Author: Matteo Merli <mmerli@apache.org>
Author: Samuel Just <sjust@salesforce.com>
Author: Nicolas Michael <nmichael@salesforce.com>
Author: Jia Zhai <zhaijia@apache.org>
Author: Sijie Guo <guosijie@gmail.com>
Author: cguttapalem <cguttapalem@salesforce.com>
Author: Ivan Kelly <ivank@apache.org>
Author: JV Jujjuri <vjujjuri@salesforce.com>
Author: Andrey Yegorov <dlg99@users.noreply.github.com>
Author: Ivan Kelly <ivan@ivankelly.net>
Author: Ethan Li <ethanopensource@gmail.com>
Author: arunlakshman <arunmuthuravi@gmail.com>
Author: Qi Wang <codingwangqi@gmail.com>
Author: Andrey Yegorov <ayegorov@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1746 from athanatos/forupstream/wip-1744-4.7, closes #1737
reddycharan pushed a commit to reddycharan/bookkeeper that referenced this issue Oct 19, 2018
ISSUE apache#1737: EntryMemTable.newEntry: always make a copy
a0144be 
Retaining a reference to that array assumes that the caller
won't reuse the array for something else -- an assumption
violated by Journal.scanJournal and probably other callers.

(@bug W-5499346@)
(@Rev cguttapalem@)
Signed-off-by: Samuel Just <sjustsalesforce.com>

Author:

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Matteo Merli <mmerli@apache.org>, Sijie Guo <sijie@apache.org>

This closes apache#1744 from athanatos/forupstream/wip-1737, closes apache#1737

(cherry picked from commit df65958)
(cherry picked from commit b75a0a3)
@athanatos athanatos mentioned this issue Oct 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@athanatos athanatos

Labels
area/bookie durability priority/important release/4.7.3 release/4.8.1 release/4.9.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@athanatos