sumitagrawl commented on code in PR #8822:
URL: https://github.com/apache/ozone/pull/8822#discussion_r2218761675
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java:
##########
@@ -86,6 +86,7 @@ public final class
ScmBlockLocationProtocolClientSideTranslatorPB
private final ScmBlockLocationProtocolPB rpcProxy;
private SCMBlockLocationFailoverProxyProvider failoverProxyProvider;
+ private ScmInfo scmInfo;
Review Comment:
declare it as volatile
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/common/BlockGroup.java:
##########
@@ -30,30 +30,23 @@
public final class BlockGroup {
private String groupID;
- @Deprecated
- private List<BlockID> blockIDs;
private List<DeletedBlock> deletedBlocks;
private BlockGroup(String groupID, List<BlockID> blockIDs,
List<DeletedBlock> deletedBlocks) {
this.groupID = groupID;
- this.blockIDs = blockIDs == null ? new ArrayList<>() : blockIDs;
this.deletedBlocks = deletedBlocks == null ? new ArrayList<>() :
deletedBlocks;
}
public List<DeletedBlock> getAllDeletedBlocks() {
return deletedBlocks;
}
- public List<BlockID> getBlockIDs() {
- return blockIDs;
- }
-
public String getGroupID() {
return groupID;
}
- public KeyBlocks getProto() {
- return deletedBlocks.isEmpty() ? getProtoForBlockID() :
getProtoForDeletedBlock();
+ public KeyBlocks getProto(boolean isDistributionEnabled) {
+ return isDistributionEnabled ? getProtoForDeletedBlock() :
getProtoForBlockID();
Review Comment:
isIncludeBlockSize can be named for isDistributionEnabled.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/common/BlockGroup.java:
##########
@@ -88,13 +81,13 @@ public static BlockGroup getFromProto(KeyBlocks proto) {
}
public static BlockGroup getFromBlockIDProto(KeyBlocks proto) {
- List<BlockID> blockIDs = new ArrayList<>();
+ List<DeletedBlock> deletedBlocks = new ArrayList<>();
Review Comment:
We can combine getFromDeletedBlockProto() as another loop here only, as will
get either getBlocksList() or getDeletedBlocksList() from OM.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -752,22 +752,14 @@ public PendingKeysDeletion getPendingDeletionKeys(
// Skip the key if the filter doesn't allow the file to be deleted.
if (filter == null || filter.apply(Table.newKeyValue(kv.getKey(),
info))) {
- BlockGroup.Builder keyBlocksBuilder =
BlockGroup.newBuilder().setKeyName(kv.getKey());
- if (isDataDistributionEnabled) {
- List<DeletedBlock> deletedBlocks =
info.getKeyLocationVersions().stream()
- .flatMap(versionLocations ->
versionLocations.getLocationList().stream()
- .map(b -> new DeletedBlock(new
BlockID(b.getContainerID(), b.getLocalID()),
- b.getLength(),
QuotaUtil.getReplicatedSize(b.getLength(), info.getReplicationConfig()))))
- .collect(Collectors.toList());
- keyBlocksBuilder.addAllDeletedBlocks(deletedBlocks);
- } else {
- List<BlockID> blockIDS = info.getKeyLocationVersions().stream()
- .flatMap(versionLocations ->
versionLocations.getLocationList().stream()
- .map(b -> new BlockID(b.getContainerID(),
b.getLocalID()))).collect(Collectors.toList());
- keyBlocksBuilder.addAllBlockIDs(blockIDS);
- }
- BlockGroup keyBlocks = keyBlocksBuilder.build();
- int keyBlockSerializedSize =
keyBlocks.getProto().getSerializedSize();
+ List<DeletedBlock> deletedBlocks =
info.getKeyLocationVersions().stream()
+ .flatMap(versionLocations ->
versionLocations.getLocationList().stream()
+ .map(b -> new DeletedBlock(new
BlockID(b.getContainerID(), b.getLocalID()),
+ b.getLength(),
QuotaUtil.getReplicatedSize(b.getLength(), info.getReplicationConfig()))))
+ .collect(Collectors.toList());
+ BlockGroup keyBlocks =
BlockGroup.newBuilder().setKeyName(kv.getKey())
+ .addAllDeletedBlocks(deletedBlocks).build();
+ int keyBlockSerializedSize =
keyBlocks.getProto(isDataDistributionEnabled).getSerializedSize();
Review Comment:
we can have getProto() and its implementation, have default as true for this
as new SCM, will not consider scm version for this as assumed to be new. and no
impact.
--
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: issues-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
