Remove APIs deprecated in 1.3.0 - #19143
Open
xiangfu0 wants to merge 1 commit into
Open
Conversation
- FunctionRegistry: remove deprecated containsFunction(String) and getFunctionInfo(String, int); migrate FastJsonPathExtractorTest to lookupFunctionInfo(canonicalize(name), numArguments) - TransformFunctionType: remove deprecated getAlternativeNames() - ScalarFunction annotation: remove deprecated isPlaceholder() attribute - PartitionDedupMetadataManager: remove deprecated checkRecordPresentOrUpdate(PrimaryKey, IndexSegment); make the DedupRecordInfo variant a plain interface method and drop the throwing override in BasePartitionDedupMetadataManager - SegmentGeneratorConfig: remove deprecated isNullHandlingEnabled()/setNullHandlingEnabled(boolean) aliases; RealtimeSegmentConfig.Builder: remove deprecated setNullHandlingEnabled(boolean) - Schema: remove deprecated clone() override - QueryContext.Builder: remove deprecated setExplain(boolean) - ControllerRequestURLBuilder: remove deprecated forSegmentsMetadataFromServer(String, String) - SegmentAssignmentUtils: remove deprecated getNumSegmentsToBeMovedPerInstance - BrokerSelectorUtils: remove deprecated getTablesCommonBrokers; drop its List-variant tests (Set-variant coverage retained) - Remove dead non-split-commit residue: ServerSegmentCompletionProtocolHandler.segmentCommit and SegmentCompletionProtocol.SegmentCommitRequest (endpoints removed in apache#14559); keep MSG_TYPE_COMMIT as the segment-completion FSM key with a clarifying comment
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19143 +/- ##
============================================
- Coverage 65.62% 65.60% -0.02%
Complexity 1423 1423
============================================
Files 3439 3439
Lines 218195 218142 -53
Branches 34669 34662 -7
============================================
- Hits 143198 143122 -76
- Misses 63443 63460 +17
- Partials 11554 11560 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a 1.3.0-cycle deprecation sweep by removing long-deprecated (and some dead) APIs across Pinot’s SPI, segment, controller, core, and client modules, updating callers/tests accordingly, and aligning the segment-completion protocol documentation with the split-commit flow.
Changes:
- Removed deprecated public APIs/aliases in
pinot-spi,pinot-segment-spi, andpinot-java-client, and migrated internal/test callers to the preferred replacements. - Simplified the dedup SPI by removing the deprecated
PrimaryKeyoverload and requiring implementations to provide theDedupRecordInfooverload directly. - Removed non-split-commit residue from segment completion (server handler + protocol request type) and refreshed
SegmentCompletionProtocoldocumentation/tests.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java | Removes deprecated String columns overload in favor of List<String> overloads. |
| pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java | Removes deprecated/incorrect Schema.clone() implementation. |
| pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java | Removes deprecated isPlaceholder annotation attribute. |
| pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/creator/SegmentGeneratorConfig.java | Removes deprecated null-handling alias accessors; keeps defaultNullHandlingEnabled accessors. |
| pinot-segment-local/src/test/java/org/apache/pinot/segment/local/dedup/BasePartitionDedupMetadataManagerTest.java | Updates dummy metadata manager to implement the now-required dedup method overload. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/RealtimeSegmentConfig.java | Removes deprecated builder alias setNullHandlingEnabled. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/dedup/PartitionDedupMetadataManager.java | Removes deprecated PrimaryKey overload and makes DedupRecordInfo overload abstract. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/dedup/BasePartitionDedupMetadataManager.java | Removes deprecated PrimaryKey overload implementation. |
| pinot-core/src/main/java/org/apache/pinot/server/realtime/ServerSegmentCompletionProtocolHandler.java | Removes dead non-split segmentCommit(...) path and associated imports. |
| pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java | Removes deprecated Builder.setExplain(boolean) in favor of ExplainMode. |
| pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java | Removes deprecated getNumSegmentsToBeMovedPerInstance in favor of getNumSegmentsToMovePerInstance. |
| pinot-common/src/test/java/org/apache/pinot/common/protocols/SegmentCompletionProtocolTest.java | Updates protocol URL test to use split-commit start request. |
| pinot-common/src/test/java/org/apache/pinot/common/function/FastJsonPathExtractorTest.java | Migrates to lookupFunctionInfo(canonicalize(name), n) after removal of deprecated lookup. |
| pinot-common/src/main/java/org/apache/pinot/common/protocols/SegmentCompletionProtocol.java | Updates protocol class Javadoc for split-commit; removes SegmentCommitRequest type; retains MSG_TYPE_COMMIT for FSM lookup. |
| pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java | Removes deprecated getAlternativeNames() alias. |
| pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java | Removes deprecated containsFunction and getFunctionInfo wrappers. |
| pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/utils/BrokerSelectorUtilsTest.java | Drops tests for removed deprecated list-returning API variant. |
| pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/utils/BrokerSelectorUtils.java | Removes deprecated getTablesCommonBrokers list-returning API. |
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
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.
Removes APIs whose deprecations shipped in the 1.3.0 cycle (2024 H2 – early 2025). Part of a series of per-release deprecation sweeps.
Every removed member was verified deprecated as of the
release-1.3.0tag and to have zero non-deprecated production callers. Full-reactortest-compile,checkstyle:check, andlicense:checkpass.Removed
FunctionRegistry.containsFunction(String)andgetFunctionInfo(String, int)→contains(canonicalize(name))/lookupFunctionInfo(canonicalize(name), n).TransformFunctionType.getAlternativeNames().ScalarFunction.isPlaceholder()annotation attribute — no@ScalarFunctionapplication anywhere set it, and nothing read it.SegmentGeneratorConfig.is/setNullHandlingEnabledandRealtimeSegmentConfig.Builder.setNullHandlingEnabled— pure aliases of thedefaultNullHandlingEnabledaccessors.Schema.clone()(undocumented shallow copy;cloneSchemaWithNameis untouched).QueryContext.Builder.setExplain(boolean)→ theExplainModeoverload.ControllerRequestURLBuilder.forSegmentsMetadataFromServer(String, String)→ theList<String>overload.SegmentAssignmentUtils.getNumSegmentsToBeMovedPerInstance→getNumSegmentsToMovePerInstance. (The removed version also carried a latent unchecked-getNPE.)BrokerSelectorUtils.getTablesCommonBrokers→getTablesCommonBrokersSet, which returns an empty set rather thannull. Its six List-variant tests were dropped; each had a parallel Set-variant test that remains.checkRecordPresentOrUpdate(PrimaryKey, IndexSegment)is gone fromPartitionDedupMetadataManager, and the survivingDedupRecordInfooverload is now a plain abstract method rather than a default that delegated into it. See the compat note below.ServerSegmentCompletionProtocolHandler.segmentCommit(...)andSegmentCompletionProtocol.SegmentCommitRequest, orphaned when Cleanup non split commit code #14559 removed the controller endpoints. These two were never annotated@Deprecated— they are dead code removed alongside their release-line neighbours.MSG_TYPE_COMMITis retained, with a comment, because it is still the FSM lookup key inSegmentCompletionManager.Also refreshes the
SegmentCompletionProtocolclass javadoc, which still described the removed single-shot commit flow, to document the actual split-commit sequence.backward-incompat
Please apply the
backward-incompatlabel.Two items deserve explicit release-note treatment:
dedupConfig.metadataManagerClass, loaded reflectively) must now implementcheckRecordPresentOrUpdate(DedupRecordInfo, IndexSegment)directly. An implementation that only overrode the deprecatedPrimaryKeyvariant previously worked via the default delegation; it will now fail withAbstractMethodErrorat ingestion time. Such implementations were already broken in practice — the base class'sPrimaryKeyoverride threwUnsupportedOperationException— but the failure mode changes.pinot-spi/pinot-segment-spi/pinot-java-clientremovals: old binaries calling e.g.Schema.clone()will getNoSuchMethodError. Replacements are listed above.No config keys, wire formats, or ZK-serialized fields are touched.