Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove alert for pre-existing new columns while merging realtime schema #16989

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

findingrish
Copy link
Contributor

@findingrish findingrish commented Sep 3, 2024

Parent issue: #14989

Currently, an alert is thrown while merging datasource schema with realtime segment schema when the datasource schema already has update columns from the delta schema.

This isn't an error condition since the datasource schema can have those columns from a different segment.

One scenario in which this can occur is when multiple replicas for a task is run.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Comment on lines 835 to 836
"Realtime schema merge: datasource [%s] is missing columns to be updated. "
+ "ExistingSignature [%s], deltaSchema [%s], missingUpdateColumns [%s].",
Copy link
Contributor

Choose a reason for hiding this comment

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

The alert description should be small. Don't include big objects like the existingSignatureor deltaSchema in it. You may add these fields separately using .addData().

existingNewColumns
).emit();
if (!missingUpdateColumns.isEmpty()) {
log.makeAlert("Datasource schema is missing columns to be updated in delta realtime segment schema.")
Copy link
Contributor

Choose a reason for hiding this comment

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

The message is confusing. Could you try to rephrase this to say exactly what went wrong and what can be done to remediate?

boolean missingUpdateColumns = false;
// new column to be added is already present in the existing schema
boolean existingNewColumns = false;
Set<String> missingUpdateColumns = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
Set<String> missingUpdateColumns = new HashSet<>();
final Set<String> missingUpdateColumns = new HashSet<>();

@kfaraz kfaraz merged commit 4e02e5b into apache:master Sep 5, 2024
93 checks passed
edgar2020 pushed a commit to edgar2020/druid that referenced this pull request Sep 5, 2024
…ma (apache#16989)

Currently, an alert is thrown while merging datasource schema with realtime
segment schema when the datasource schema already has update columns from the delta schema.

This isn't an error condition since the datasource schema can have those columns from a different segment.

One scenario in which this can occur is when multiple replicas for a task is run.
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
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.

3 participants