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 >