Skip to content

Add API to fetch conflicting task locks#16799

Merged
AmatyaAvadhanula merged 7 commits intoapache:masterfrom
AmatyaAvadhanula:add_api_conflicting_locks
Jul 30, 2024
Merged

Add API to fetch conflicting task locks#16799
AmatyaAvadhanula merged 7 commits intoapache:masterfrom
AmatyaAvadhanula:add_api_conflicting_locks

Conversation

@AmatyaAvadhanula
Copy link
Copy Markdown
Contributor

@AmatyaAvadhanula AmatyaAvadhanula commented Jul 25, 2024 •
edited
Loading

Description

Add a new API to view the active task locks for a given set of (datasource, priority, interval list).
This can be useful to:

  1. View lock contention
  2. Observe existence of concurrent locks
  3. Augment and replace the existing /lockedIntervals API
  4. Miscellaneous debugging

Changes

  1. Remove the existing deprecated API to view locked intervals: /lockedIntervals
  2. Augment lock filter policy to take a list of intervals
  3. Add an API to view task locks given a set of filters instead: /activeLocks
  4. Add tests and modify existing ones accordingly

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.

@AmatyaAvadhanula AmatyaAvadhanula requested a review from kfaraz July 25, 2024 04:56
@AmatyaAvadhanula AmatyaAvadhanula marked this pull request as ready for review July 29, 2024 05:02
kfaraz
kfaraz reviewed Jul 29, 2024
View reviewed changes
Comment thread indexing-service/src/main/java/org/apache/druid/indexing/common/TaskLock.java Outdated
Comment thread server/src/main/java/org/apache/druid/metadata/TaskLockInfo.java Outdated
Comment thread server/src/main/java/org/apache/druid/metadata/TaskLockInfo.java Outdated
Comment thread server/src/main/java/org/apache/druid/server/http/TaskLockResponse.java Outdated
Comment thread server/src/main/java/org/apache/druid/rpc/indexing/OverlordClient.java Outdated
Comment thread indexing-service/src/main/java/org/apache/druid/indexing/overlord/TaskQueryTool.java Outdated
Comment thread indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java Outdated
Comment thread indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
github-advanced-security[bot]
github-advanced-security AI found potential problems Jul 29, 2024
View reviewed changes
Comment thread indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLockboxTest.java Fixed
kfaraz
kfaraz approved these changes Jul 29, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Thanks, @AmatyaAvadhanula !
+1 after CI passes.

Comment thread indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
@Produces(MediaType.APPLICATION_JSON)
@ResourceFilters(StateResourceFilter.class)
public Response getDatasourceLockedIntervalsV2(List<LockFilterPolicy> lockFilterPolicies)
public Response getActiveLocks(List<LockFilterPolicy> lockFilterPolicies)
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Super Nit: If you reverse the order of these two methods in this class file, the diff might become smaller/simpler. 🙂
(consider doing this only if you are also fixing some other build issue in this PR)

@AmatyaAvadhanula AmatyaAvadhanula merged commit 92a40d8 into apache:master Jul 30, 2024
@kfaraz kfaraz deleted the add_api_conflicting_locks branch July 30, 2024 06:11
sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
@AmatyaAvadhanula @sreemanamala
Add API to fetch conflicting task locks (apache#16799)
ff1e5c7 
* Add API to fetch conflicting active locks
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
@kfaraz kfaraz mentioned this pull request Oct 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kfaraz kfaraz kfaraz approved these changes

Assignees

No one assigned

Labels

Area - Ingestion

Projects

None yet

Milestone

31.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@AmatyaAvadhanula @kfaraz @github-advanced-security