Skip to content

Remove APIs deprecated in 1.0.0 - #19140

Merged
xiangfu0 merged 1 commit into
apache:masterfrom
xiangfu0:xiangfu0/remove-deprecated-1.0
Aug 2, 2026
Merged

Remove APIs deprecated in 1.0.0#19140
xiangfu0 merged 1 commit into
apache:masterfrom
xiangfu0:xiangfu0/remove-deprecated-1.0

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Part of #19147.

Removes APIs whose deprecations shipped in the 1.0.0 cycle (2023 H1). Part of a series of per-release deprecation sweeps.

All removed members were verified to have zero non-deprecated production callers. Full-reactor test-compile, checkstyle:check, and license:check pass.

Removed

  • OnHeapGuavaBloomFilterCreator 4-arg constructor. Its own TODO named Complete index spi #10184 as the exit criteria, which merged long ago. Both remaining call sites already use the 5-arg form.
  • PinotHelixResourceManager.deleteSchema(Schema) — also returned false on its success path. Every remaining deleteSchema call site passes a String.
  • PinotHelixResourceManager.getSchemaForTableConfig(TableConfig) — see the correction below; this one is a 1.4.0-era deprecation, not 1.0.0.
  • Stale class javadoc in PinotSegmentRestletResource: a "Deprecated APIs" section listing five /tables/{tableName}/segments* endpoints that no longer exist anywhere in the codebase.

Staged: /segments/{tableName}/select

SegmentAdminClient.selectSegments is repointed from the deprecated GET /segments/{tableName}/select to GET /segments/{tableName}, which is an exact functional superset (same response shape List<Map<TableType, List<String>>>, same timestamp parsing and validation, equivalent authorization — both resolve to action GetSegment with TargetType.TABLE).

The deprecated endpoint is intentionally retained in this PR. pinot-java-client has been shipping a selectSegments that calls /select throughout the 1.5.x and 1.6.x train; since controllers upgrade before client fleets, deleting it now would 404 every already-deployed client. It can be deleted once this client migration has had a release to propagate.

Two behavior changes worth calling out

Both make the client match its own documented contract, but they are observable:

  1. selectSegments(..., excludeReplacedSegments=false) now returns replaced segments. The old /select endpoint never read that query parameter — it hardcoded getSegmentsFor(tableNameWithType, true, ...) — so the flag the client had always been sending was silently ignored and replaced segments were always excluded. The new endpoint honors it. Callers passing false will see a larger result set. The only in-repo caller (LaunchBackfillIngestionJobCommand) passes true and is unaffected.
  2. The database header is now honored for segment selection, consistent with every other /segments/* endpoint. /select ignored it.

Correction: one item is in the wrong release bucket

Thanks to @yashmayya for catching this. getSchemaForTableConfig was not deprecated in 1.0.0 — it was still a live method with real fallback logic at release-1.3.0, and @Deprecated first appears at release-1.4.0, added by #15333 (Mar 2025) when schema became mandatory for all tables.

I re-verified against the tags and confirm his reading:

tag deprecated?
release-1.0.0 → release-1.3.0 no
release-1.4.0 → release-1.5.1 yes

I then re-ran the same check against every member removed across all five PRs in this series (does it carry @Deprecated at release-1.3.0?) — this is the only one that was mis-bucketed. The root cause was my own method: I dated each group by git-blaming one representative line and assumed neighbouring members shared its vintage, which is exactly wrong for a method deprecated two years after its neighbour.

I've kept the removal here and called it out explicitly in the commit message rather than moving it, since it still has genuine runway (1.4.0, 1.5.0, 1.5.1) and no remaining callers — but say the word if you'd rather I pull it out and hold it for a 1.4.0 sweep.

Not removed

POST /instances/{instanceName}/state was in the original sweep list but stays: InstanceAdminClient.setInstanceState is a live non-deprecated caller, and the POST handler supports a drop operation the replacement PUT handler does not.

backward-incompat

Please apply the backward-incompat label. The removed PinotHelixResourceManager methods are source-breaking for controller plugins, and the removed bloom-filter constructor is source-breaking for out-of-tree index code. Release note should carry the two selectSegments behavior changes above.

@xiangfu0 xiangfu0 added the backward-incompat Introduces a backward-incompatible API or behavior change label Aug 1, 2026
@codecov-commenter

codecov-commenter commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.63%. Comparing base (dc95530) to head (e033693).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #19140   +/-   ##
=========================================
  Coverage     65.62%   65.63%           
  Complexity     1423     1423           
=========================================
  Files          3439     3439           
  Lines        218195   218230   +35     
  Branches      34669    34679   +10     
=========================================
+ Hits         143198   143237   +39     
- Misses        63443    63446    +3     
+ Partials      11554    11547    -7     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 65.63% <100.00%> (+<0.01%) ⬆️
temurin 65.63% <100.00%> (+<0.01%) ⬆️
unittests 65.63% <100.00%> (+<0.01%) ⬆️
unittests1 57.02% <ø> (+0.01%) ⬆️
unittests2 37.95% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes long-deprecated APIs (from the 1.0.0 deprecation cycle) across core/controller code and updates the Java admin client to use the non-deprecated segments listing endpoint, aligning client behavior with controller semantics while keeping the deprecated controller endpoint for existing deployed clients.

Changes:

  • Removed deprecated OnHeapGuavaBloomFilterCreator 4-arg constructor and deprecated PinotHelixResourceManager schema methods.
  • Cleaned up stale “Deprecated APIs” class Javadoc in PinotSegmentRestletResource.
  • Repointed SegmentAdminClient.selectSegments from GET /segments/{tableName}/select to GET /segments/{tableName} and updated the unit test accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/bloom/OnHeapGuavaBloomFilterCreator.java Removes deprecated compatibility constructor.
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java Removes deprecated schema APIs (deleteSchema(Schema), getSchemaForTableConfig).
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java Removes stale Javadoc section listing deprecated endpoints that no longer exist.
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/admin/SegmentAdminClient.java Switches selectSegments to call the non-deprecated /segments/{tableName} endpoint.
pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/admin/PinotAdminClientTest.java Updates verification to assert the new /segments/{tableName} request path.

@yashmayya

Copy link
Copy Markdown
Contributor

One of these looks like it's in the wrong bucket: PinotHelixResourceManager.getSchemaForTableConfig wasn't deprecated in 1.0.0.

At release-1.3.0 it's still a live method with real fallback logic (look up the schema by raw table name, then fall back to schemaName in the validation config):

/**
 * Find schema with same name as rawTableName. If not found, find schema using schemaName in validationConfig.
 * For OFFLINE table, it is possible that schema was not uploaded before creating the table. Hence for OFFLINE,
 * this method can return null.
 */
@Nullable
public Schema getSchemaForTableConfig(TableConfig tableConfig) {

@Deprecated first shows up at release-1.4.0, added by #15333 (Mar 2025) when schema became mandatory for all tables. Checking the tags:

release-1.0.0  not deprecated
release-1.1.0  not deprecated
release-1.2.0  not deprecated
release-1.3.0  not deprecated
release-1.4.0  deprecated

So it's carried the annotation through 1.4.0, 1.5.0 and 1.5.1 — real runway, and there are no remaining callers, so no objection to removing it. But it's a shorter window than the "1.0.0" heading implies, and anyone later auditing when a given API was deprecated would get the wrong answer from the PR title. Might be worth moving it to a 1.4.0-era PR, or just calling it out separately in the release notes.

Everything else in this PR checks out as @Deprecated at release-1.0.0.

Sunsets deprecated APIs whose deprecations shipped in the 1.0.0 cycle
(2023 H1). All removed members had zero non-deprecated production callers.

- OnHeapGuavaBloomFilterCreator: remove the 4-arg constructor; its own TODO
  named apache#10184 as the exit criteria, which has long since merged
- PinotHelixResourceManager: remove deleteSchema(Schema), which also
  returned false on its success path
- PinotHelixResourceManager: remove getSchemaForTableConfig(TableConfig). Note
  this one is NOT a 1.0.0-era deprecation - it was deprecated in 1.4.0 by
  apache#15333 (Mar 2025), when schema became mandatory for all tables. It is
  included here rather than held for a 1.4.0 sweep because it has carried the
  annotation through 1.4.0, 1.5.0 and 1.5.1 and has no remaining callers
- PinotSegmentRestletResource: drop the stale "Deprecated APIs" section from
  the class javadoc; none of the endpoints it listed still exist

Also repoints SegmentAdminClient.selectSegments from the deprecated
"GET /segments/{tableName}/select" to "GET /segments/{tableName}". The
deprecated endpoint is retained this release so that already-shipped 1.5.x
and 1.6.x clients keep working against an upgraded controller; it can be
deleted once those clients have had a release to migrate.

Two behavior changes follow from the repoint, both of which make the client
match its documented contract:
- selectSegments(..., excludeReplacedSegments=false) now returns replaced
  segments. The old endpoint ignored the flag and always excluded them.
- The database header is now honored for segment selection, consistent with
  the other /segments endpoints.
@xiangfu0
xiangfu0 force-pushed the xiangfu0/remove-deprecated-1.0 branch from 6156ed0 to e033693 Compare August 2, 2026 20:44
@xiangfu0

xiangfu0 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@yashmayya good catch, you're right and I've confirmed it independently.

getSchemaForTableConfig is still live with its fallback logic at release-1.3.0; @Deprecated first appears at release-1.4.0, from #15333 as you say. So the "1.0.0" heading is wrong for this one member.

Worth naming the root cause, because it could have hidden more than one error: I dated each group by git-blaming a single representative line and assuming its neighbours shared that vintage. Here I blamed deleteSchema (deprecated 2023, correctly 1.0.0-era) and inherited that date for getSchemaForTableConfig sitting right below it — which was actually deprecated two years later.

So I re-ran the check properly across the whole series: for every member removed in all five PRs, does it carry @Deprecated at release-1.3.0? This is the only one that fails. Everything else is correctly bucketed.

I've kept the removal here and called the real vintage out explicitly in the commit message, taking your "call it out separately" option — it's carried the annotation through 1.4.0, 1.5.0 and 1.5.1 and has no remaining callers. Happy to pull it out and hold it for a 1.4.0 sweep instead if you'd rather the buckets stay strictly clean; just say the word.

Separately, note the Pinot Binary Compatibility Check red X on #19139, #19141 and #19143 — that's japicmp correctly flagging the intended pinot-spi removals, and I've listed the exact violations in each PR body. Since there's no allowlist or label bypass in the workflow, I'd appreciate direction on whether you'd prefer those merged over the failing check, or a separate PR adding a sanctioned escape hatch.

@xiangfu0
xiangfu0 merged commit ae84165 into apache:master Aug 2, 2026
12 checks passed
@xiangfu0
xiangfu0 deleted the xiangfu0/remove-deprecated-1.0 branch August 2, 2026 22:12
xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Aug 3, 2026
Documents the upgrade impact of apache/pinot#19140: SegmentAdminClient
endpoint migration, newly honored replaced-segment and database filters,
and removed extension APIs. Validation: git diff --check.

Co-authored-by: Xiang Fu <xiangfu@xiangs-macbook-pro-1.wyvern-sun.ts.net>
@xiangfu0

xiangfu0 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Documentation follow-up: pinot-contrib/pinot-docs#956

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Introduces a backward-incompatible API or behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants