Skip to content

[server] Fix NPE when processing table registration change#3431

Open
raoluSmile wants to merge 2 commits into
apache:mainfrom
raoluSmile:fix_table-registration-change-npe
Open

[server] Fix NPE when processing table registration change#3431
raoluSmile wants to merge 2 commits into
apache:mainfrom
raoluSmile:fix_table-registration-change-npe

Conversation

@raoluSmile
Copy link
Copy Markdown

What is the purpose of the change

This PR fixes a NullPointerException in
CoordinatorEventProcessor.processTableRegistrationChange.

When processing a TableRegistrationChangeEvent, the coordinator needs the old
TableInfo to get the current schema info and build the new TableInfo.
However, the old TableInfo may be unavailable if the event is stale or the
coordinator context is incomplete.

The previous code only checked whether tableId was null, but
CoordinatorContext#getTableIdByPath returns TableInfo.UNKNOWN_TABLE_ID when
the table path is not found. As a result, an unknown table could still continue
to getTableInfoById, get null, and fail at oldTableInfo.getSchemaInfo().

Brief change log

  • Treat TableInfo.UNKNOWN_TABLE_ID as an unregistered table when processing
    table registration change events.
  • Skip table registration change events if the old TableInfo is not available
    in CoordinatorContext.
  • Add regression tests for stale/incomplete coordinator context states.

Tests

Added two regression tests in CoordinatorEventProcessorTest:

  • testTableRegistrationChangeWithUnknownTable

    Covers the case where a table registration change event is processed after the
    table path is no longer present in CoordinatorContext.

    CoordinatorContext#getTableIdByPath returns TableInfo.UNKNOWN_TABLE_ID
    for this case. The previous tableId == null check did not catch it, so the
    processor continued and eventually dereferenced a null oldTableInfo.

  • testTableRegistrationChangeWithMissingTableInfo

    Covers the case where the table path to table id mapping exists, but the
    corresponding TableInfo is not available in CoordinatorContext.

    This represents an incomplete coordinator context state. Since
    TableRegistration#toTableInfo needs the old schema info, the event cannot be
    applied safely without the old TableInfo.

Before this fix, both tests failed with an NPE at
oldTableInfo.getSchemaInfo(). After this fix, the stale/incomplete events are
skipped safely.

./mvnw -pl fluss-server -am \
  -Dtest=CoordinatorEventProcessorTest#testTableRegistrationChangeWithUnknownTable+testTableRegistrationChangeWithMissingTableInfo \
  -DfailIfNoTests=false test

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

  Skip stale or incomplete table registration change events when the
  table path is unknown or the old TableInfo is missing from the
  coordinator context.

  Add regression tests for unknown tables and missing table info to
  cover the previous oldTableInfo.getSchemaInfo() NPE.
Copy link
Copy Markdown
Contributor

@loserwang1024 loserwang1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your pr, I have left some comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants