Remove deprecated _segmentPushType and _segmentPushFrequency from TableConfigBuilder#18472
Open
Akanksha-kedia wants to merge 2 commits into
Open
Conversation
…leConfigBuilder
The fields, their constants, and the deprecated setter methods have been removed from
TableConfigBuilder. The authoritative path for these values is now
IngestionConfig -> BatchIngestionConfig -> segmentIngestionType/segmentIngestionFrequency,
which is enforced by TableConfigUtils.repairTableConfig() for any legacy persisted configs.
Callers updated:
- HelixExternalViewBasedQueryQuotaManagerTest: remove redundant .setSegmentPushType("APPEND")
calls — APPEND is implicit and irrelevant to quota logic
- MultiStageEngineIntegrationTest: same
- TimeBoundaryManagerTest: set segmentPushFrequency directly on the validation config to
simulate a legacy table config for the time-boundary frequency tests
- TableConfigUtilsTest.testConvertFromLegacyTableConfig: set push type/frequency directly
on the validation config, matching how legacy configs are actually stored on disk
Akanksha-kedia
force-pushed
the
fix-remove-deprecated-segment-push-fields
branch
from
aff9746 to
e8614a7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18472 +/- ##
============================================
- Coverage 63.68% 63.68% -0.01%
Complexity 1684 1684
============================================
Files 3262 3266 +4
Lines 199826 199836 +10
Branches 31031 31023 -8
============================================
- Hits 127264 127256 -8
- Misses 62414 62429 +15
- Partials 10148 10151 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xiangfu0
reviewed
May 13, 2026
Contributor
xiangfu0
left a comment
xiangfu0
left a comment
There was a problem hiding this comment.
Found one high-signal compatibility issue; see inline comment.
…ite-through IngestionConfigUtils.getSegmentIngestionType/Frequency() still fall back to validationConfig.getSegmentPushType/Frequency() when BatchIngestionConfig is absent (IngestionConfigUtils.java:204,224). Turning the builder setters into silent no-ops while that fallback exists is a silent behavioral break: callers that pass REFRESH get APPEND with no error. Restore the private fields, setter logic, and validationConfig write-through so existing callers continue to get the table config they asked for. The methods remain @deprecated pointing to IngestionConfig. Actual removal requires a coordinated change that also removes the IngestionConfigUtils legacy fallback.
Contributor
Author
|
@xiangfu0 please help to review |
Akanksha-kedia
force-pushed
the
fix-remove-deprecated-segment-push-fields
branch
from
89e9c26 to
469afbc
Compare
Jackie-Jiang
added
cleanup
Contributor
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
This is not the correct fix. These 2 fields are moved into BatchIngestionConfig. We need to set the correct property instead
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #17072 (supersedes and expands the original PR per reviewer feedback from @xiangfu0).
The
@Deprecated_segmentPushTypeand_segmentPushFrequencyfields, their associated private constants (DEFAULT_SEGMENT_PUSH_TYPE,REFRESH_SEGMENT_PUSH_TYPE), the deprecated setter methods (setSegmentPushType,setSegmentPushFrequency), and their forwarding inbuild()have all been removed fromTableConfigBuilder.The authoritative path for these values is
IngestionConfig → BatchIngestionConfig → segmentIngestionType / segmentIngestionFrequency.TableConfigUtils.repairTableConfig()already migrates any legacy persisted configs (read from ZK) that carry these values inSegmentsValidationAndRetentionConfig.Callers updated
HelixExternalViewBasedQueryQuotaManagerTest.setSegmentPushType("APPEND")calls — irrelevant to quota logicMultiStageEngineIntegrationTest.setSegmentPushType("APPEND")callTimeBoundaryManagerTestsegmentPushFrequencydirectly on the validation config to simulate a legacy table configTableConfigUtilsTest.testConvertFromLegacyTableConfigconvertFromLegacyTableConfigmigrates them correctlyTest plan
TableConfigUtilsTest.testConvertFromLegacyTableConfigpasses — migration of legacy validation-config push type/frequency still worksTimeBoundaryManagerTestpasses — push frequency is still propagated correctlyHelixExternalViewBasedQueryQuotaManagerTestpasses — quota logic unaffected