On Wed, Feb 7, 2018 at 11:30 PM, Venkateswara Rao Jujjuri <jujj...@gmail.com
> wrote:

> On Tue, Feb 6, 2018 at 11:00 PM, Sijie Guo <guosi...@gmail.com> wrote:
>
> > On Tue, Feb 6, 2018 at 9:37 PM Venkateswara Rao Jujjuri <
> jujj...@gmail.com
> > >
> > wrote:
> >
> > > We have bunch of code in doAsyncCloseInternal() to handle
> > > MetadataVersionException.
> > > I am wondering if there is a valid case where we need to handle this.
> > Stay
> > > with me. :)
> > >
> > > When do we get this error?
> > >
> > > 1. When the client tried to write the metadata twice. Same metadata
> twice
> > > because of the client tried twice and the first attempt made it. (retry
> > > logic in bk). If so, the error is benign. (Though I have a question on
> > > metadata.isNewerThan(newMeta) logic which I will ask towards the end.)
> > >
> > > 2. Close might be happening for two reasons. 1. Application initiated
> 2.
> > > Internal initiated because of a hard error on the write path. We really
> > > don't need to fail with version mismatch for internal closes, as it
> > really
> > > doesn't matter, because our intention is to fail the write and we would
> > > anyway. Just close the local handle we are done.
> > > In the case of the application initiated close, the version mismatch
> can
> > > happen only if the fencing logic took over and changed some metadata.
> But
> > > we are guaranteed to protected up until the LAC  of the writer anyway.
> > > Since the leger is getting closed, it really doesn't matter if the ZK
> has
> > > more updated metadata version.
> > >
> > > So my question is, do we really need to handle MetadataVersionException
> > on
> > > write?
> > >
> > >  My next question is do we have anything fishy in the isNewerThan
> logic?
> > >
> > >     boolean isNewerThan(LedgerMetadata newMeta) {
> > >         if (null == version) {
> > >             return false;
> > >         }
> > >         return Version.Occurred.AFTER == version.compare(newMeta.
> > version);
> > >     }
> > > ---
> > >             MetadataVersion mv = (MetadataVersion) v;
> > >             int res = version - mv.version;
> > >             if (res == 0) {
> > >                 return Occurred.CONCURRENTLY;
> > >             } else if (res < 0) {
> > >                 return Occurred.BEFORE;
> > >             } else {
> > >                 return Occurred.AFTER;
> > >             }
> > > ---
> > >
> > > Since newMetadata is newer in this case, its version is going to be
> > higher
> > > hence res is -ve, and this is returning  Occurred.BEFORE, but it must
> be
> > > Occurred.AFTER as the metadata update happened 'after' what we have. Or
> > the
> > > isNewerThan logic is inverted.
> > >
> > > if (!metadata.isNewerThan(newMeta)
> > >          && !metadata.isConflictWith(newMeta)) {
> > > } else {
> > > LOG.warn("Conditional update ledger metadata for ledger {} failed.",
> > >                                                     ledgerId);
> > >                                             cb.closeComplete(rc,
> > > LedgerHandle.this, ctx);
> > > }
> > >
> > > In this case, even though local metadata is NOT newer than newMeta
> > because
> > > of the logic we are failing.
> > >
> > >
> > > So two questions:
> > > 1. Do we really need to handle MetadataVersionMismatch in close?
> >
> >
> > I think the take was: when you got metadata version mismatch, you don’t
> > really know who updated the metadata, could be another writer, could be
> > retried from same writer. So we popped up the error to application and
> let
> > applications decide what to do. I think this model works fine, since it
> is
> > on closing path, application can choose ignore close error. Do you see
> any
> > bad effects on your applications?
> >
> >
> I just want to check if there is anything we need to be worried about
> if application/client ignores version mismatch error on close. Looks like
> its ok. Right?
>

Yeah that's okay


>
> >
> > > 2. Is this metadata.isNewerTha() logic is corretc?
> > >
> > >
> > The logic seems to be fine with me. A.isNewerThan(B), is checking if A’s
> > version is after B’s version, no?
> >
> >
> I got my logic inverted; never mind.
>
>
> > Sijie
> >
> >
> > --
> > > Jvrao
> > > ---
> > > First they ignore you, then they laugh at you, then they fight you,
> then
> > > you win. - Mahatma Gandhi
> > >
> >
>
>
>
> --
> Jvrao
> ---
> First they ignore you, then they laugh at you, then they fight you, then
> you win. - Mahatma Gandhi
>

Reply via email to