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