HDDS-8010. Improve DN warning message when getBlock does not find the block

[GAOJHIHCYUAN]

What changes were proposed in this pull request?

adding the log message information about block id and logged at WARN level

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8010

How was this patch tested?

CI tests

[dineshchitlangia]

+1 LGTM, pending CI

[chungen0126]

I think this should be still logged at DEBUG level if it is under the isDebugEnabled() condition.

[GAOJHIHCYUAN]

thanks for your comment
I commit new version that if it is under the condition still logged at DEBUG level.
if it's not under the condition logged at Warn level.

[adoroszlai]

@GAOJHIHCYUAN have you observed the updated message in the log?

[adoroszlai]

request.getGetBlock() is only applicable to GetBlock requests. Also, not all commands are related to blocks anyway.

Looking at HDDS-8010, I think the goal is to change the message in:

ozone/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

Lines 353 to 364 in 4afa68e

	privateBlockDatagetBlockByID(DBHandledb, BlockIDblockID,
	KeyValueContainerDatacontainerData) throwsIOException {
	StringblockKey = containerData.getBlockKey(blockID.getLocalID());
	BlockDatablockData = db.getStore().getBlockDataTable().get(blockKey);
	if (blockData == null) {
	thrownewStorageContainerException(NO_SUCH_BLOCK_ERR_MSG,
	NO_SUCH_BLOCK);
	}
	returnblockData;
	}

[GAOJHIHCYUAN]

request.getGetBlock() is only applicable to GetBlock requests. Also, not all commands are related to blocks anyway.

Looking at HDDS-8010, I think the goal is to change the message in:

ozone/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java

Lines 353 to 364 in 4afa68e

	privateBlockDatagetBlockByID(DBHandledb, BlockIDblockID,
	KeyValueContainerDatacontainerData) throwsIOException {
	StringblockKey = containerData.getBlockKey(blockID.getLocalID());
	BlockDatablockData = db.getStore().getBlockDataTable().get(blockKey);
	if (blockData == null) {
	thrownewStorageContainerException(NO_SUCH_BLOCK_ERR_MSG,
	NO_SUCH_BLOCK);
	}
	returnblockData;
	}

public class StorageContainerException extends IOException {
private final ContainerProtos.Result result;
private final BlockID blockID = getLocalID();
public BlockID getLocalID() {return blockID;}

if (log.isDebugEnabled()) {
log.debug(logInfo, request.getCmdType(), request.getTraceID(),
ex.getLocalID(),
ex.getMessage(), ex.getResult().getValueDescriptor().getName(), ex);
}
} else {
log.warn(logInfo, request.getCmdType(), request.getTraceID(),
ex.getLocalID(),
ex.getMessage(), ex.getResult().getValueDescriptor().getName(), ex);
}

because the log was logged when StorageContainerException occurred
my opinion was adding the getLocalID() in StorageContainerException.java
and then we can call function in getBlockByID() getting the block ID.
Idk is it a good soultion.
thanks you so much

@GAOJHIHCYUAN have you observed the updated message in the log?

No,I haven't
I will try how could I observe the updated message.
sorry,i was a newbie. I'm still learning.
thanks you so much.

[ashishkumar50]

You can update message in getBlockByID while throwing StorageContainerException to print the blockID. Can you update the PR?

[GAOJHIHCYUAN]

You can update message in getBlockByID while throwing StorageContainerException to print the blockID. Can you update the PR?

okay,I will update the PR.
appreciate

[adoroszlai]

Thanks again @GAOJHIHCYUAN for working on this. Will you update the patch to make the change in BlockManagerImpl#getBlockByID instead of the generic logAndReturnError method?

[adoroszlai]

/pending

[github-actions]

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

/pending

[GAOJHIHCYUAN]

Thanks again @GAOJHIHCYUAN for working on this. Will you update the patch to make the change in BlockManagerImpl#getBlockByID instead of the generic logAndReturnError method?

I will update the patch.
Thanks you so much.