xichen01 commented on code in PR #10946:
URL: https://github.com/apache/ozone/pull/10946#discussion_r3785999438
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java:
##########
@@ -171,28 +172,48 @@ public AllocatedBlock allocateBlock(final long size,
// configured, it will be of type StorageType.DISK and therefore belong
// to a StorageTier.DISK tier, so for old clients the write process is
// unchanged if the Datanode Volume configuration is not changed.
- StorageTier storageTier = StorageTier.getDefaultTier();
- ContainerInfo containerInfo = writableContainerFactory.getContainer(
- size, replicationConfig, owner, excludeList, storageTier);
+ boolean isFallBack = false;
+ ContainerInfo containerInfo = null;
+ try {
+ containerInfo = writableContainerFactory.getContainer(
+ size, replicationConfig, owner, excludeList,
storagePolicy.getCreationTier());
+ } catch (IOException e) {
+ if (allowFallbackStoragePolicy &&
storagePolicy.getCreationFallbackTier() != StorageTier.EMPTY) {
+ // TODO StoragePolicy should It should be distinguished in detail
which exceptions can try to fallback
+ isFallBack = true;
+ containerInfo = writableContainerFactory.getContainer(size,
replicationConfig, owner,
+ excludeList, storagePolicy.getCreationFallbackTier());
+ } else {
+ throw e;
+ }
Review Comment:
I think the current implementation, which throws an exception, is a better
approach.
Because `BlockManagerImpl#allocateBlock` does not catch exceptions from
`writableContainerFactory.getContainer` previous, the exceptions from
`getContainer` are thrown directly. We should maintain this logic, meaning that
if `getContainer` fails and cannot be re-tried an exception should be thrown.
L187 rethrows the exception when a fallback is not possible; this is an
exception that should have been thrown in the first place.
The internal version was originally implemented in version 1.2, when
`getContainer` would return null instead of throws an exception upon failure.
After we switched to version 1.4, we haven't changed the processing logic here.
--
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]
