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

Make numCorePartitions as 0 for tombstones #15379

Merged

Conversation

abhishekrb19
Copy link
Contributor

@abhishekrb19 abhishekrb19 commented Nov 15, 2023

This PR changes the core partitions for tombstones to 0, so they can exist independently if they're deleted or marked as unused without affecting availability of other appended segments in the same co-partition space. This is particularly important in the following scenarios:

  1. Concurrent replace and append to a datasource containing tombstones and data segments in the same time chunk (also, related coordinator duty: Clean up duty for non-overlapping eternity tombstones #15281)
  2. If a user drops a tombstone segment and there are other data segments appended to it, the data segments should still be available

Release note

Tombstone segments now have 0 core partitions instead of 1. This means they can be dropped or removed independently without affecting availability of other appended segments in the same co-partition space. Note that removing tombstones with 1 core partition (prior to this change) that contain appended segments in the partition space can make the appended segments unavailable.


Key changed/added classes in this PR
  • TombstoneShardSpec.java
  • IndexerSQLMetadataStorageCoordinatorTest.java

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 unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@abhishekrb19 abhishekrb19 marked this pull request as ready for review November 16, 2023 09:26
@abhishekrb19 abhishekrb19 changed the title WIP: Make numCorePartitions as 0 in TombstoneShardSpec Make numCorePartitions as 0 for tombstones Nov 16, 2023
Copy link
Contributor

@AmatyaAvadhanula AmatyaAvadhanula left a comment

Choose a reason for hiding this comment

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

Thanks @abhishekrb19! LGTM

@kfaraz
Copy link
Contributor

kfaraz commented Nov 17, 2023

Conceptually, the change makes sense to me but taking another pass just to be sure that it doesn't break any existing assumption in the code. Also, there are a couple of test failures.

@abhishekrb19
Copy link
Contributor Author

Thanks for the reviews!

@abhishekrb19 abhishekrb19 merged commit 470c8ed into apache:master Nov 20, 2023
87 checks passed
@abhishekrb19 abhishekrb19 deleted the make_tombstone_core_partitions_0 branch November 20, 2023 17:42
writer-jill pushed a commit to writer-jill/druid that referenced this pull request Nov 20, 2023
* Make numCorePartitions as 0 in the TombstoneShardSpec.

* fix up test

* Add tombstone core partition tests

* review comment

* Need to register the test shard type to make jackson happy
yashdeep97 pushed a commit to yashdeep97/druid that referenced this pull request Dec 1, 2023
* Make numCorePartitions as 0 in the TombstoneShardSpec.

* fix up test

* Add tombstone core partition tests

* review comment

* Need to register the test shard type to make jackson happy
@LakshSingla LakshSingla added this to the 29.0.0 milestone Jan 29, 2024
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