loserwang1024 commented on code in PR #3431:
URL: https://github.com/apache/fluss/pull/3431#discussion_r3354246286
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java:
##########
@@ -756,7 +756,7 @@ private void
processTableRegistrationChange(TableRegistrationChangeEvent event)
// Skip if the table is not yet registered in coordinator context.
// Should not happen in normal cases.
- if (tableId == null) {
+ if (tableId == null || tableId == TableInfo.UNKNOWN_TABLE_ID) {
Review Comment:
`tableId == TableInfo.UNKNOWN_TABLE_ID` is enough,
coordinatorContext.getTableIdByPath never return null
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java:
##########
@@ -765,6 +765,14 @@ private void
processTableRegistrationChange(TableRegistrationChangeEvent event)
}
TableInfo oldTableInfo = coordinatorContext.getTableInfoById(tableId);
+ if (oldTableInfo == null) {
+ LOG.warn(
+ "Table {} with id {} is not loaded in coordinator context,
"
+ + "skip processing table registration change.",
+ tablePath,
+ tableId);
+ return;
Review Comment:
Even if oldTableInfo is null, we should not return directly, otherwise
`coordinatorContext.putTableInfo(newTableInfo);` will not be executed.
However, I don't think we need this check here. The reason why a
NullPointerException was thrown previously was because
org.apache.fluss.server.coordinator.CoordinatorContext#getTableIdByPath
returned TableInfo.UNKNOWN_TABLE_ID when the entry did not exist. Now that
you've added a prior validation check, this issue should no longer occur.
--
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]
