You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Charan Reddy G <re...@gmail.com> on 2017/04/27 22:27:00 UTC
Issue with entryLogger.leastUnflushedLogId, when preallocatedLogId
rolls over Int.MAX
Hey,
With Bookkeeper-833 Bug (
https://github.com/apache/bookkeeper/commit/da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf),
entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId rolls
over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we set
"leastUnflushedLogId = flushedLogId + 1;", so it makes leastUnflushedLogId
also to roll over. But this affects the
GarbageCollectorThread.extractMetaFromEntryLogs logic. This method extracts
EntryLogMetadata from the newly rotated entrylogs, but when rollover
happens this logic seems to be broken
in GarbageCollectorThread.java
protected Map<Long, EntryLogMetadata>
extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap) {
// Extract it for every entry log except for the current one.
// Entry Log ID's are just a long value that starts at 0 and
increments
// by 1 when the log fills up and we roll to a new one.
long curLogId = entryLogger.getLeastUnflushedLogId();
<------when rollover happens, this will start from 1
boolean hasExceptionWhenScan = false;
for (long entryLogId = scannedLogId; entryLogId < curLogId;
entryLogId++) { <------- because of "entryLogId < curLogId" condition it
will skip the newly rotated logs
Thanks,
Charan
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Sijie Guo <gu...@gmail.com>.
I think Bookkeeper-833 is irrelevant, no? Even it is Long.MAX, it would
still have the roll over problem.
The question is - is this really an issue? When roll over happen, it might
cause an entry log file won't be garbage collected or compacted in this
lifecycle of the bookie. But it doesn't actually impact correctness, right?
- Sijie
On Thu, Apr 27, 2017 at 3:27 PM, Charan Reddy G <re...@gmail.com>
wrote:
> Hey,
>
> With Bookkeeper-833 Bug (https://github.com/apache/bookkeeper/commit/
> da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf), entryLogId in EntryLogger is
> capped at Int.MAX, so preallocatedLogId rolls over to 0 after reaching
> Int.MAX. In EntryLogger.flushRotatedLogs we set "leastUnflushedLogId =
> flushedLogId + 1;", so it makes leastUnflushedLogId also to roll over. But
> this affects the GarbageCollectorThread.extractMetaFromEntryLogs logic.
> This method extracts EntryLogMetadata from the newly rotated entrylogs, but
> when rollover happens this logic seems to be broken
>
> in GarbageCollectorThread.java
>
> protected Map<Long, EntryLogMetadata> extractMetaFromEntryLogs(Map<Long,
> EntryLogMetadata> entryLogMetaMap) {
> // Extract it for every entry log except for the current one.
> // Entry Log ID's are just a long value that starts at 0 and
> increments
> // by 1 when the log fills up and we roll to a new one.
> long curLogId = entryLogger.getLeastUnflushedLogId();
> <------when rollover happens, this will start from 1
> boolean hasExceptionWhenScan = false;
> for (long entryLogId = scannedLogId; entryLogId < curLogId;
> entryLogId++) { <------- because of "entryLogId < curLogId" condition it
> will skip the newly rotated logs
>
> Thanks,
> Charan
>
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Sijie Guo <gu...@gmail.com>.
On Wed, May 3, 2017 at 4:16 AM, Charan Reddy G <re...@gmail.com>
wrote:
>
> "The question is - is this really an issue? When roll over happen, it
> might cause an entry log file won't be garbage collected or compacted in
> this lifecycle of the bookie. But it doesn't actually impact correctness,
> right?"
>
> Yes it is not correctness issue. But the chances of those entrylogs
> getting garbage collected are very slim (even after Bookie restart).
> extractMetaFromEntryLogs will extract metadata from those entrylogs next
> time when entryLogger.getLeastUnflushedLogId() reaches close to Int.MAX
> but not rolled over again.
>
I agreed with this is an issue that we need to address. I am just trying to
clarify the severity of this issue.
- Sijie
>
> Thanks,
> Charan
>
>
> On Fri, Apr 28, 2017 at 12:11 AM, Enrico Olivelli <eo...@gmail.com>
> wrote:
>
>> Il ven 28 apr 2017, 09:05 Sijie Guo <gu...@gmail.com> ha scritto:
>>
>> > Hi Enrico,
>> >
>> > Let's try to figure things out in the email thread before create a JIRA.
>> >
>>
>> Got it, sorry
>> Enrico
>>
>> >
>> > - Sijie
>> >
>> > On Thu, Apr 27, 2017 at 11:43 PM, Enrico Olivelli <eo...@gmail.com>
>> > wrote:
>> >
>> >> Thank you Charan,
>> >> Can you create a JIRA?
>> >> Do you already have a fix?
>> >>
>> >> Il ven 28 apr 2017, 00:27 Charan Reddy G <re...@gmail.com> ha
>> >> scritto:
>> >>
>> >>> Hey,
>> >>>
>> >>> With Bookkeeper-833 Bug (
>> >>>
>> >>> https://github.com/apache/bookkeeper/commit/da1a2fa6b19ddcdb
>> a68834147bf6afbe5bf90cbf
>> >>> ),
>> >>> entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId
>> >>> rolls
>> >>> over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we
>> set
>> >>> "leastUnflushedLogId = flushedLogId + 1;", so it makes
>> >>> leastUnflushedLogId
>> >>> also to roll over. But this affects the
>> >>> GarbageCollectorThread.extractMetaFromEntryLogs logic. This method
>> >>> extracts
>> >>> EntryLogMetadata from the newly rotated entrylogs, but when rollover
>> >>> happens this logic seems to be broken
>> >>>
>> >>> in GarbageCollectorThread.java
>> >>>
>> >>> protected Map<Long, EntryLogMetadata>
>> >>> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata>
>> entryLogMetaMap) {
>> >>> // Extract it for every entry log except for the current one.
>> >>> // Entry Log ID's are just a long value that starts at 0 and
>> >>> increments
>> >>> // by 1 when the log fills up and we roll to a new one.
>> >>> long curLogId = entryLogger.getLeastUnflushedLogId();
>> >>> <------when rollover happens, this will start from 1
>> >>> boolean hasExceptionWhenScan = false;
>> >>> for (long entryLogId = scannedLogId; entryLogId < curLogId;
>> >>> entryLogId++) { <------- because of "entryLogId < curLogId"
>> condition
>> >>> it
>> >>> will skip the newly rotated logs
>> >>>
>> >>> Thanks,
>> >>> Charan
>> >>>
>> >> --
>> >>
>> >>
>> >> -- Enrico Olivelli
>> >>
>> >
>> > --
>>
>>
>> -- Enrico Olivelli
>>
>
>
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Charan Reddy G <re...@gmail.com>.
"The question is - is this really an issue? When roll over happen, it might
cause an entry log file won't be garbage collected or compacted in this
lifecycle of the bookie. But it doesn't actually impact correctness, right?"
Yes it is not correctness issue. But the chances of those entrylogs getting
garbage collected are very slim (even after Bookie restart).
extractMetaFromEntryLogs
will extract metadata from those entrylogs next time when
entryLogger.getLeastUnflushedLogId()
reaches close to Int.MAX but not rolled over again.
Thanks,
Charan
On Fri, Apr 28, 2017 at 12:11 AM, Enrico Olivelli <eo...@gmail.com>
wrote:
> Il ven 28 apr 2017, 09:05 Sijie Guo <gu...@gmail.com> ha scritto:
>
> > Hi Enrico,
> >
> > Let's try to figure things out in the email thread before create a JIRA.
> >
>
> Got it, sorry
> Enrico
>
> >
> > - Sijie
> >
> > On Thu, Apr 27, 2017 at 11:43 PM, Enrico Olivelli <eo...@gmail.com>
> > wrote:
> >
> >> Thank you Charan,
> >> Can you create a JIRA?
> >> Do you already have a fix?
> >>
> >> Il ven 28 apr 2017, 00:27 Charan Reddy G <re...@gmail.com> ha
> >> scritto:
> >>
> >>> Hey,
> >>>
> >>> With Bookkeeper-833 Bug (
> >>>
> >>> https://github.com/apache/bookkeeper/commit/
> da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf
> >>> ),
> >>> entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId
> >>> rolls
> >>> over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we
> set
> >>> "leastUnflushedLogId = flushedLogId + 1;", so it makes
> >>> leastUnflushedLogId
> >>> also to roll over. But this affects the
> >>> GarbageCollectorThread.extractMetaFromEntryLogs logic. This method
> >>> extracts
> >>> EntryLogMetadata from the newly rotated entrylogs, but when rollover
> >>> happens this logic seems to be broken
> >>>
> >>> in GarbageCollectorThread.java
> >>>
> >>> protected Map<Long, EntryLogMetadata>
> >>> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap)
> {
> >>> // Extract it for every entry log except for the current one.
> >>> // Entry Log ID's are just a long value that starts at 0 and
> >>> increments
> >>> // by 1 when the log fills up and we roll to a new one.
> >>> long curLogId = entryLogger.getLeastUnflushedLogId();
> >>> <------when rollover happens, this will start from 1
> >>> boolean hasExceptionWhenScan = false;
> >>> for (long entryLogId = scannedLogId; entryLogId < curLogId;
> >>> entryLogId++) { <------- because of "entryLogId < curLogId"
> condition
> >>> it
> >>> will skip the newly rotated logs
> >>>
> >>> Thanks,
> >>> Charan
> >>>
> >> --
> >>
> >>
> >> -- Enrico Olivelli
> >>
> >
> > --
>
>
> -- Enrico Olivelli
>
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Enrico Olivelli <eo...@gmail.com>.
Il ven 28 apr 2017, 09:05 Sijie Guo <gu...@gmail.com> ha scritto:
> Hi Enrico,
>
> Let's try to figure things out in the email thread before create a JIRA.
>
Got it, sorry
Enrico
>
> - Sijie
>
> On Thu, Apr 27, 2017 at 11:43 PM, Enrico Olivelli <eo...@gmail.com>
> wrote:
>
>> Thank you Charan,
>> Can you create a JIRA?
>> Do you already have a fix?
>>
>> Il ven 28 apr 2017, 00:27 Charan Reddy G <re...@gmail.com> ha
>> scritto:
>>
>>> Hey,
>>>
>>> With Bookkeeper-833 Bug (
>>>
>>> https://github.com/apache/bookkeeper/commit/da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf
>>> ),
>>> entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId
>>> rolls
>>> over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we set
>>> "leastUnflushedLogId = flushedLogId + 1;", so it makes
>>> leastUnflushedLogId
>>> also to roll over. But this affects the
>>> GarbageCollectorThread.extractMetaFromEntryLogs logic. This method
>>> extracts
>>> EntryLogMetadata from the newly rotated entrylogs, but when rollover
>>> happens this logic seems to be broken
>>>
>>> in GarbageCollectorThread.java
>>>
>>> protected Map<Long, EntryLogMetadata>
>>> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap) {
>>> // Extract it for every entry log except for the current one.
>>> // Entry Log ID's are just a long value that starts at 0 and
>>> increments
>>> // by 1 when the log fills up and we roll to a new one.
>>> long curLogId = entryLogger.getLeastUnflushedLogId();
>>> <------when rollover happens, this will start from 1
>>> boolean hasExceptionWhenScan = false;
>>> for (long entryLogId = scannedLogId; entryLogId < curLogId;
>>> entryLogId++) { <------- because of "entryLogId < curLogId" condition
>>> it
>>> will skip the newly rotated logs
>>>
>>> Thanks,
>>> Charan
>>>
>> --
>>
>>
>> -- Enrico Olivelli
>>
>
> --
-- Enrico Olivelli
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Sijie Guo <gu...@gmail.com>.
Hi Enrico,
Let's try to figure things out in the email thread before create a JIRA.
- Sijie
On Thu, Apr 27, 2017 at 11:43 PM, Enrico Olivelli <eo...@gmail.com>
wrote:
> Thank you Charan,
> Can you create a JIRA?
> Do you already have a fix?
>
> Il ven 28 apr 2017, 00:27 Charan Reddy G <re...@gmail.com> ha
> scritto:
>
>> Hey,
>>
>> With Bookkeeper-833 Bug (
>> https://github.com/apache/bookkeeper/commit/
>> da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf),
>> entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId rolls
>> over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we set
>> "leastUnflushedLogId = flushedLogId + 1;", so it makes leastUnflushedLogId
>> also to roll over. But this affects the
>> GarbageCollectorThread.extractMetaFromEntryLogs logic. This method
>> extracts
>> EntryLogMetadata from the newly rotated entrylogs, but when rollover
>> happens this logic seems to be broken
>>
>> in GarbageCollectorThread.java
>>
>> protected Map<Long, EntryLogMetadata>
>> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap) {
>> // Extract it for every entry log except for the current one.
>> // Entry Log ID's are just a long value that starts at 0 and
>> increments
>> // by 1 when the log fills up and we roll to a new one.
>> long curLogId = entryLogger.getLeastUnflushedLogId();
>> <------when rollover happens, this will start from 1
>> boolean hasExceptionWhenScan = false;
>> for (long entryLogId = scannedLogId; entryLogId < curLogId;
>> entryLogId++) { <------- because of "entryLogId < curLogId" condition
>> it
>> will skip the newly rotated logs
>>
>> Thanks,
>> Charan
>>
> --
>
>
> -- Enrico Olivelli
>
Re: Issue with entryLogger.leastUnflushedLogId, when
preallocatedLogId rolls over Int.MAX
Posted by Enrico Olivelli <eo...@gmail.com>.
Thank you Charan,
Can you create a JIRA?
Do you already have a fix?
Il ven 28 apr 2017, 00:27 Charan Reddy G <re...@gmail.com> ha
scritto:
> Hey,
>
> With Bookkeeper-833 Bug (
>
> https://github.com/apache/bookkeeper/commit/da1a2fa6b19ddcdba68834147bf6afbe5bf90cbf
> ),
> entryLogId in EntryLogger is capped at Int.MAX, so preallocatedLogId rolls
> over to 0 after reaching Int.MAX. In EntryLogger.flushRotatedLogs we set
> "leastUnflushedLogId = flushedLogId + 1;", so it makes leastUnflushedLogId
> also to roll over. But this affects the
> GarbageCollectorThread.extractMetaFromEntryLogs logic. This method extracts
> EntryLogMetadata from the newly rotated entrylogs, but when rollover
> happens this logic seems to be broken
>
> in GarbageCollectorThread.java
>
> protected Map<Long, EntryLogMetadata>
> extractMetaFromEntryLogs(Map<Long, EntryLogMetadata> entryLogMetaMap) {
> // Extract it for every entry log except for the current one.
> // Entry Log ID's are just a long value that starts at 0 and
> increments
> // by 1 when the log fills up and we roll to a new one.
> long curLogId = entryLogger.getLeastUnflushedLogId();
> <------when rollover happens, this will start from 1
> boolean hasExceptionWhenScan = false;
> for (long entryLogId = scannedLogId; entryLogId < curLogId;
> entryLogId++) { <------- because of "entryLogId < curLogId" condition it
> will skip the newly rotated logs
>
> Thanks,
> Charan
>
--
-- Enrico Olivelli