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

Add API to return automatic compaction config history #13699

Merged
merged 6 commits into from
Jan 23, 2023

Conversation

suneet-s
Copy link
Contributor

@suneet-s suneet-s commented Jan 20, 2023

Description

Add a new API to return the history of changes to automatic compaction config history to make it easy for users to see what changes have been made to their auto-compaction config.

The API is scoped per dataSource to allow users to triage issues with an individual dataSource. The API responds with a list of configs when there is a change to either the settings that impact all auto-compaction configs on a cluster or the dataSource in question.

I had considered adding another API to expose the entire compaction config history as it was stored in the audit table, however I chose against doing this, as I didn't think users would want the ability to see this global view.

Release note

New: You can now get the history of changes to automatic compaction config for a dataSource via an API.


Key changed/added classes in this PR
  • DataSourceCompactionConfigHistory#add - Ensures that only changes that affect a particular dataSource's auto-compaction config are added to the history
  • CoordinatorCompactionConfigsResource#getCompactionConfigHistory - the API endpoint

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 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.

@suneet-s suneet-s requested a review from maytasm January 21, 2023 02:07
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

This is very useful, @suneet-s !
+1 after CI passes

Comment on lines 74 to 88
if (newEntry != null || (current != null && !hasDataSourceCompactionConfig)) {
if (newEntry == null) {
newEntry = new DatasourceCompactionConfigAuditEntry(
new DatasourceCompactionConfigAuditEntry.GlobalCompactionConfig(
coordinatorCompactionConfig.getCompactionTaskSlotRatio(),
coordinatorCompactionConfig.getMaxCompactionTaskSlots(),
coordinatorCompactionConfig.isUseAutoScaleSlots()
),
null,
auditInfo,
auditTime
);
}
auditEntries.push(newEntry);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe simplify this for readability:

    if (newEntry != null) {
      auditEntries.push(newEntry);
    } else if (current != null && !hasDataSourceCompactionConfig) {
        newEntry = ...
        auditEntries.push(newEntry);
    }

/**
* A DTO containing audit information for compaction config for a datasource.
*/
public class DatasourceCompactionConfigAuditEntry
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Rename to DataSourceCompactionConfigAuditEntry with a capitalized S in DataSource, similar to rest of the classes here.

@suneet-s suneet-s merged commit 016c881 into apache:master Jan 23, 2023
@suneet-s suneet-s deleted the compaction-history branch January 23, 2023 21:23
count
);
} else {
auditEntries = auditManager.fetchAuditHistory(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can a user pass in both interval and count?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the pattern in CoordinatorDynamicConfigsResource#getDatasourceRuleHistory where it looks like we ignore the count silently if the interval is specified.

abhagraw pushed a commit to abhagraw/druid that referenced this pull request Feb 8, 2023
Add a new API to return the history of changes to automatic compaction config history to make it easy for users to see what changes have been made to their auto-compaction config.

The API is scoped per dataSource to allow users to triage issues with an individual dataSource. The API responds with a list of configs when there is a change to either the settings that impact all auto-compaction configs on a cluster or the dataSource in question.
@clintropolis clintropolis added this to the 26.0 milestone Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants