sodonnel commented on PR #10109:
URL: https://github.com/apache/ozone/pull/10109#issuecomment-4379142286
A problem with enforcing atomic update of the containerData within a
Container object, is that the Container object has the RW lock, and it allows
for the ContainerData object to be fetched without a lock. Then the
ContainerData object can also be mutated without a lock, or mutated by another
thread even when the Container object is locked.
What about DeleteBlocksCommandHandler, which has this code which locks the
Container Object AFTER the containerData object has already been extracted from
it and then proceeds to pass that down into lower methods to change it.
```
case KeyValueContainer:
KeyValueContainer keyValueContainer = (KeyValueContainer)cont;
KeyValueContainerData containerData =
keyValueContainer.getContainerData();
if (keyValueContainer.
writeLockTryLock(tryLockTimeoutMs, TimeUnit.MILLISECONDS)) {
try {
String schemaVersion = containerData
.getSupportedSchemaVersionOrDefault();
if (getSchemaHandlers().containsKey(schemaVersion)) {
schemaHandlers.get(schemaVersion).handle(containerData, tx);
} else {
throw new UnsupportedOperationException(
"Only schema version 1,2,3 are supported.");
}
} finally {
keyValueContainer.writeUnlock();
}
txResultBuilder.setContainerID(containerId)
.setSuccess(true);
} else {
lockAcquisitionFailed = true;
txResultBuilder.setContainerID(containerId)
.setSuccess(false);
}
break;
```
This feels very fragile too and we haven't made any changes to this class -
does it need the same check after lock?
It feels like all these container data fields should be retrieved though the
Container object and that is should check a valid lock is in place before
returning it.
The problem is, containerData is used in 100's of places across the code
base - mostly in tests but over 100 in non-test code too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
