Skip to content

[Do Not Merge] For review only#3846

Closed
jaydeepkumar1984 wants to merge 132 commits intoapache:trunkfrom
jaydeepkumar1984:trunk_cep_37_jan_28_2025_for_review
Closed

[Do Not Merge] For review only#3846
jaydeepkumar1984 wants to merge 132 commits intoapache:trunkfrom
jaydeepkumar1984:trunk_cep_37_jan_28_2025_for_review

Conversation

@jaydeepkumar1984
Copy link
Copy Markdown
Contributor

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

jaydeepkumar1984 and others added 30 commits January 28, 2025 17:20
jaydeep1984 and others added 29 commits January 28, 2025 17:29
Summary of impacting changes:

* RepairTokenRangeSplitter is now the default in favor of
  FixedSplitTokenRangeSplitter.

* number_of_subranges moved from a repair_type_override/global config
  to a property of FixedSplitTokenRangeSplitter.

* [get|set]autorepairconfig usability changes

Detailed breakdown of changes:

0. IAutoRepairTokenRangeSplitter changes:

  * Move RepairType from an parameter in getRepairAssignments to
    a parameter in the constructor for implementations.  This was done
    because the RepairType is always the same for a splitter instance.

  * Add setParameter and getParameter methods which are used by
    setautorepairconfig and getautorepairconfig to dynamically update
    splitter configuration.

1. [get|set]autorepairconfig changes:

  * getautorepairconfig output now shows property names instead of
    human readable names (e.g. repair_check_interval instead of
    repair eligibility check interval). This was done to make it more
    intuitive to know what properties to use for setautorepairconfig.

  * getautorepairconfig and setautorepairconfig now support viewing
    and changing splitter properties, e.g.:

    setautorepairconfig token_range_splitter.max_bytes_per_schedule 500GiB -t full

2. RepairTokenRangeSplitter changes:

  * Renames RepairRangeSplitter to RepairTokenRangeSplitter and makes it
    the default implementation.

  * Establishes defaults for each repair type to be sensible.

  * Improve javadocs detailing the primary goal of the splitter, its
    configuration and its defaults and the justifications for using them.

  * Rename variables to be consistent with their setting names.

3. FixedSplitTokenRangeSplitter changes:

  * Renames DefaultAutoRepairTokenSplitter to
    FixedSplitTokenRangeSplitter as it is no longer the default.

  * Move number_of_subranges from a global config to a property for
    this splitter.

4. RepairAssignmentIterator

  * Refactored common code from both splitter implementations into
    RepairAssignmentIterator with the aim to reduce the amount of
    boiler plate custom splitter implementations need to implement.

5. Test changes

  * Fix AutoRepairParameterizedTest to use fixed splitter so we get a
    deterministic repair plan.

  * Allow splitter to be changed programmatically, only expect it to be
    used for tests.

  * Rename CassandraSreamReceiverTest and fix it

    Whether streaming cdc/mvs into commitlogs was previously dependent
    on system properties;  update the test to account for the new
    yaml properties.

  * Fix dtest after CASSANDRA-20160

    The introduction of repair_task_min_duration causes repairs to take
    cumulatively longer for a node than 2 minutes.  To resolve this,
    set that to 0s, and also enable repair_by_keyspace and set subranges
    to 1 to reduce the overall number of repairs.

Patch by Andy Tolbert; reviewed by ___ for CASSANDRA-20179
* Move curly brackets to new line
* Remove unused config declaration in constructor
Also remove redundant AutoRepairConfig.RepairType for RepairType
in AutoRepairConfig.
* Promote node not being present in gossip to warn
* Clean up NUMBER_OF_SUBRANGES doc
* Simplify default map parameter parsing in both splitters
* Doc cleanup and make RepairAssignmentIterator fully public
* Remove unnecessary getters and setters in Splitters
* Handle case where ColumnFamilyStore not retrievable, in this case
  return no assignments as we can assume deleted.
* Always return a RepairAssignment for a table, even if empty, in
  RepairTokenRangeSplitter as node may have missed writes
* Add set|getParameters tests
* Update AutoRepairParameterizedTest to use RepairTokenRangeSplitter
* Move no-split specific test to FixedSplitTokenRangeSplitterTest
Updates partitions_per_assignment to not be based on
repair_session_max_tree_depth which is deprecated.  Instead, just
use 2^20.

Also updates documentation around partitions_per_assignment and cleans
up some warnings in RepairTokeRangeSplitter.

patch by Andy Tolbert; reviewed by Jaydeep Chovatia for CASSANDRA-20231
Creating repair sessions by table can create an overwhelming amount of
repairs, especially with vnodes.

If a repair assignment is too big (> 64 tables by default, or > 200GB
for full/50GB for incremental) RepairTokenRangeSplitter will already
split into multiple repair assignments.

Adjusts repair_by_keyspace to default to true.

patch by Andy Tolbert; reviewed by Jaydeep Chovatia for CASSANDRA-20232
Patch by Francisco Guerrero; reviewed by TBD for CASSANDRA-20185
…ation in the AutoRepairServiceMBean definition
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.

6 participants