In our last meeting, we briefly discussed about options of handling
negative long ledgerids in LongeHierarchicalLedgerManager. Mainly by using
the unsigned long value (20 digits) for determining the ledger zNode path.
I see couple of problems with that

(signed long)         ->     (unsigned long)
-9223372036854775808  ->  "09223372036854775808"
 9223372036854775807  ->  "09223372036854775807"

now both these ledger znodes would be under the same parent zNode
0922/3372/0368/5477

gc method in ScanAndCompareGarbageCollector, for LedgerRange
(0922/3372/0368/5477) the following logic would break. Because we would get
mix of positive and negative ledgerids in this ledgerrange and
scanandcompare logic with bkActiveLedgers (ledgerStorage.getActiveLedgers)
will not work.


            while(ledgerRangeIterator.hasNext()) {
                LedgerRange lRange = ledgerRangeIterator.next();

                Long start = lastEnd + 1;
                Long end = lRange.end();
                if (!ledgerRangeIterator.hasNext()) {
                    end = Long.MAX_VALUE;
                }

                Iterable<Long> subBkActiveLedgers =
bkActiveLedgers.subSet(start, true, end, true);

and also with ledgerrange with all negative ledgerids we need to do some
tweaking for the 'start' and the 'end'.

So in summary, I feel its risky to use unsigned long for determining the
ledger zNode path, since we used signed long for ledgerids all across the
codebase and this inconsistency can cause issue in some other areas as well.

So I'm more convinced to use the signed long for determining the ledger
zNode path and let the LongHierarchicalLedgerManager/Iterator deal with
negative ledgerids accordingly (for eg. deal with znode with '-' while
doing comparison and iteration). So it makes scope of the change limited to
LongHierarchicalLedgerManager rather than applying patches to other areas.

(ledger id)      ->  (znode path)
-9223372036854775808  ->  "-922/3372/0368/5477/5808"
 9223372036854775807  ->  "0922/3372/0368/5477/5807"

Thanks,
Charan


On Wed, Oct 26, 2016 at 10:55 PM, Sijie Guo <si...@apache.org> wrote:

> On Wed, Oct 26, 2016 at 3:49 PM, Matteo Merli <mme...@apache.org> wrote:
>
> > On Wed, Oct 26, 2016 at 11:45 AM Venkateswara Rao Jujjuri <
> > jujj...@gmail.com>
> > wrote:
> >
> > > - Ledgers are unique across multiple clusters. Useful if storage tiers
> > with
> > > different stores are employed.
> > >
> >
> > For this you could combine the ledgerId with another 64bit id, that could
> > encode the rest of the required informations ( storage tier, cluster, ..
> )
> >
>
> +1 on this idea
>
>
> >
> >
> > > - No centralized id creation - Allows client to give the name instead
> of
> > > server generating name on create.
> > >
> >
> > This should be already possible with your changes in 4.4, right?
> >
> > Wouldn't be enough to combine the 64bit ledgerId with an additional id
> that
> > doesn't need to flow through BK ?
> >
>

Reply via email to