fix(clustering): ensure data plane config hash is never nil #11917
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
This makes the test less complicated and easier to debug on failure.
flrgh
force-pushed
the
fix/clustering-empty-config-hash
branch
from
e53f614
to
017629c
Compare
The previous logic defaulted the config_hash to nil when it was
detected to be an empty string. This can cause update_sync_status() to
fail, because config_hash is a required attribute:
> 2023/11/03 17:13:30 [debug] 4052224#0: *150 [lua] connector.lua:560: execute(): SQL query throw error: ERROR: null value in column "config_hash" of relation "clustering_data_planes" violates not-null constraint
> Failing row contains (4fb29006-8db1-48bb-b68c-34b582e1d91a, soup, 127.0.0.1, 2023-11-04 00:13:30+00, null, 2023-11-18 00:13:30.799+00, 3.6.0, filter_set_incompatible, 2023-11-04 00:13:30+00, {})., close connection
> 2023/11/03 17:13:30 [notice] 4052224#0: *150 [lua] init.lua:275: upsert(): ERROR: null value in column "config_hash" of relation "clustering_data_planes" violates not-null constraint
This change addresses the problem from two angles:
1. when empty, config_hash is set to the default DECLARATIVE_EMPTY_CONFIG_HASH
constant instead of nil
2. an additional guard was added to the dp reader thread, which checks the
length of ping frame data and returns an error if it is not a proper
config hash
flrgh
force-pushed
the
fix/clustering-empty-config-hash
branch
from
017629c
to
5b420ed
Compare
chronolaw
approved these changes
Nov 7, 2023
bungle
approved these changes
Nov 7, 2023
hanshuebner
approved these changes
Nov 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Bug
The previous logic defaulted the config_hash to nil when it was detected to be an empty string. This can cause update_sync_status() to fail, because config_hash is a required attribute:
Why was config_hash an empty string? Because we blindly update this variable from the data received from ping frames from the data plane
The Fix
This change addresses the problem from two angles:
Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary.There is a user-facing docs PRN/AIssue reference
KAG-2967