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?
2. Is this metadata.isNewerTha() logic is corretc?

-- 
Jvrao
---
First they ignore you, then they laugh at you, then they fight you, then
you win. - Mahatma Gandhi

Reply via email to