Skip to content

Conversation

@ctubbsii
Copy link
Member

@ctubbsii ctubbsii commented Dec 23, 2023

This fixes a bug in the compaction properties to ensure the replacement
property is always preferred over the deprecated open.max compaction
property when it is set.

Add tests for maxOpen to override open.max:

  • Adds a test to ensure that setting the maxOpen option for a compaction
    service will override the deprecated open.max property if set
  • Condenses helper methods in planner tests
  • Uses CompactionPlannerInitParams for tests instead of custom test code
  • Adds test case for default compaction service used with deprecated
    property
  • Removes hardcoded maxOpen value with reference to default property
    value
  • Modifies the getFullyQualifiedOption to return the correct path for
    the <service>.planner.opts. properties

This is a reapplication of #4092 after it was reverted, to use
SiteConfiguration for testing overrides rather than modifications to
ConfigurationCopy

Changes made by ctubbsii that diverge from #4092:

  • Update commit log message to add detail and format it
  • Omit changes to ConfigurationCopy, including changes in Update isPropertySet to check parent config #4112, which
    is now OBE, to add a parent to preserve its role as a simple "flat"
    configuration object for testing and simple operations
  • Use SiteConfiguration with overrides, instead of ConfigurationCopy
    with a parent, to test override behavior for
    DefaultCompactionPlannerTest

Co-authored-by: Christopher Tubbs ctubbsii@apache.org

asfgit pushed a commit that referenced this pull request Dec 23, 2023
asfgit pushed a commit that referenced this pull request Dec 23, 2023
This reverts commit 67fd967.

Reverting in favor of #4117
@ctubbsii ctubbsii force-pushed the 4092-rebase branch 2 times, most recently from ab1101b to 586b714 Compare December 23, 2023 08:21
@ctubbsii ctubbsii changed the title 4092 rebase Fix bug in compaction props (reapply #4092) Dec 23, 2023
@ctubbsii ctubbsii changed the base branch from 2.1 to main December 23, 2023 08:30
@ctubbsii ctubbsii changed the base branch from main to 2.1 December 23, 2023 08:30
This fixes a bug in the compaction properties to ensure the replacement
property is always preferred over the deprecated open.max compaction
property when it is set.

Add tests for maxOpen to override open.max:
* Adds a test to ensure that setting the maxOpen option for a compaction
  service will override the deprecated `open.max` property if set
* Condenses helper methods in planner tests
* Uses CompactionPlannerInitParams for tests instead of custom test code
* Adds test case for default compaction service used with deprecated
  property
* Removes hardcoded maxOpen value with reference to default property
  value
* Modifies the getFullyQualifiedOption to return the correct path for
  the `<service>.planner.opts.` properties

This is a reapplication of apache#4092 after it was reverted, to use
SiteConfiguration for testing overrides rather than modifications to
ConfigurationCopy

Changes made by ctubbsii that diverge from apache#4092:
* Update commit log message to add detail and format it
* Omit changes to ConfigurationCopy, including changes in apache#4112, which
  is now OBE, to add a parent to preserve its role as a simple "flat"
  configuration object for testing and simple operations
* Use SiteConfiguration with overrides, instead of ConfigurationCopy
  with a parent, to test override behavior for
  DefaultCompactionPlannerTest

Co-authored-by: Christopher Tubbs <ctubbsii@apache.org>
@ctubbsii
Copy link
Member Author

@ddanielr This should be merged into 2.1 only if whoever merges it in is prepared to subsequently merge the updated 2.1 into main. This will reapply #4092, which I reverted because of the non-trivial merge conflicts trying to merge into main
from 2.1 and because of the issues with SiteConfiguration/ConfigurationCopy mentioned above (and in Slack), which I've already fixed in this PR.

@ddanielr ddanielr merged commit fdd73fb into apache:2.1 Dec 27, 2023
@ctubbsii ctubbsii deleted the 4092-rebase branch December 28, 2023 07:01
@ctubbsii ctubbsii modified the milestones: 3.1.0, 2.1.3 Jul 12, 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.

2 participants