Tracking issue for an audit-driven cleanup of Pinot's deprecated public surface.
Background
Master carries 371 @Deprecated annotations, some dating to 2016. I audited all of them, plus unmarked legacy config keys, stale feature flags, and obsolete code paths, to establish for each one: what replaced it, whether any non-deprecated production code still calls it, and whether removal is safe with respect to ZooKeeper-serialized config, wire formats, on-disk segment format, and third-party SPI implementations.
Approach
Rather than one large sweep, the removals are split one PR per deprecating release line. Each PR removes only APIs whose deprecation shipped in that release, so a reviewer can reason about a single deprecation window, and a future reader auditing "when was X deprecated?" gets a truthful answer from the PR that removed it.
Every removed member was verified to have zero non-deprecated production callers. Each branch is a single commit, green on full-reactor test-compile, checkstyle:check, and license:check on JDK 25.
Open PRs
Roughly 1,140 lines of dead surface. All are labelled backward-incompat; each PR body carries the removed→replacement mapping and the operator-facing notes for its own scope.
Note on the Binary Compatibility Check
Pinot Binary Compatibility Check fails by design on #19139, #19141 and #19143. It runs japicmp with --error-on-binary-incompatibility over pinot-spi / pinot-segment-spi, and removing public members from those modules is precisely the intent of this work. The exact violations are enumerated in each PR body so they can be eyeballed against the intended removals. It is not a required check, so it does not block merge.
Deliberately excluded
These came up during the audit and were not removed, each for a specific reason. Recording them here so they are not re-proposed without the context:
| Item |
Why it stays |
POST /segments/{tableName}/delete |
The client migration to DELETE /segments/{tableName} is unsafe as written — see the bugs section below. Needs the client fixed first. |
SegmentLocks static factories |
ConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletes still calls the deprecated static; migrating changes lock scoping from a global cache to per-TableDataManager locks, which is a concurrency change, not a deprecation cleanup. |
TableAndSchemaConfig |
Still the request-body DTO of PinotUpsertRestletResource, and its JSON shape differs from TableConfigs; swapping would break the public REST contract. |
ControllerGauge.OFFLINE_TABLE_ESTIMATED_SIZE |
Still emitted by StorageQuotaChecker. |
POST /instances/{instanceName}/state |
InstanceAdminClient.setInstanceState is a live caller, and the POST handler supports a drop operation the replacement PUT handler does not. |
HeaderAdder ThirdEye URI-rewrite hack |
Never @Deprecated, runs @PreMatching on every controller request, and has no test coverage — an unverifiable live behavior change that should not ride along with mechanical deletions. Deserves its own PR. |
Follow-up work
Staged endpoint deletions. Three endpoints had their callers migrated in the PRs above but were intentionally left in place, so that an N-1 component still works against an N component during a rolling upgrade. They can be deleted once these PRs have shipped in a release:
Remaining release lines. The audit identified further removable APIs deprecated in 1.4.0, 1.5.0, and within the unreleased 1.6.0 cycle. Following the same one-PR-per-release pattern:
New deprecations to add. The audit also found legacy surface that is not yet marked at all. Worth its own discussion rather than bundling here — notably the HttpClient auth-less overloads (documented as deprecated but never annotated), the silent old→new config-key fallbacks that log nothing when the legacy key is used, and the pre-fieldConfigList index-declaration fields on IndexingConfig.
Bugs found in passing
These exist on master today and are unrelated to the PRs above. Filing separately, listed here for traceability:
SegmentAdminClient DELETE path: an empty/null segment list omits the segments query parameter, which the controller interprets as "delete all segments of the table"SegmentAdminClient DELETE path: multi-segment deletes comma-join into a single parameter value that never matches the controller's repeated @QueryParam("segments") binding, so they silently no-opTenantAdminClient.changeTenantState issues PUT /tenants/{tenantName}/metadata, but that path declares only @GET and @POST — it can only ever return 405
Tracking issue for an audit-driven cleanup of Pinot's deprecated public surface.
Background
Master carries 371
@Deprecatedannotations, some dating to 2016. I audited all of them, plus unmarked legacy config keys, stale feature flags, and obsolete code paths, to establish for each one: what replaced it, whether any non-deprecated production code still calls it, and whether removal is safe with respect to ZooKeeper-serialized config, wire formats, on-disk segment format, and third-party SPI implementations.Approach
Rather than one large sweep, the removals are split one PR per deprecating release line. Each PR removes only APIs whose deprecation shipped in that release, so a reviewer can reason about a single deprecation window, and a future reader auditing "when was X deprecated?" gets a truthful answer from the PR that removed it.
Every removed member was verified to have zero non-deprecated production callers. Each branch is a single commit, green on full-reactor
test-compile,checkstyle:check, andlicense:checkon JDK 25.Open PRs
Roughly 1,140 lines of dead surface. All are labelled
backward-incompat; each PR body carries the removed→replacement mapping and the operator-facing notes for its own scope.Note on the Binary Compatibility Check
Pinot Binary Compatibility Checkfails by design on #19139, #19141 and #19143. It runs japicmp with--error-on-binary-incompatibilityoverpinot-spi/pinot-segment-spi, and removing public members from those modules is precisely the intent of this work. The exact violations are enumerated in each PR body so they can be eyeballed against the intended removals. It is not a required check, so it does not block merge.Deliberately excluded
These came up during the audit and were not removed, each for a specific reason. Recording them here so they are not re-proposed without the context:
POST /segments/{tableName}/deleteDELETE /segments/{tableName}is unsafe as written — see the bugs section below. Needs the client fixed first.SegmentLocksstatic factoriesConcurrentMapPartitionUpsertMetadataManagerForConsistentDeletesstill calls the deprecated static; migrating changes lock scoping from a global cache to per-TableDataManagerlocks, which is a concurrency change, not a deprecation cleanup.TableAndSchemaConfigPinotUpsertRestletResource, and its JSON shape differs fromTableConfigs; swapping would break the public REST contract.ControllerGauge.OFFLINE_TABLE_ESTIMATED_SIZEStorageQuotaChecker.POST /instances/{instanceName}/stateInstanceAdminClient.setInstanceStateis a live caller, and the POST handler supports adropoperation the replacement PUT handler does not.HeaderAdderThirdEye URI-rewrite hack@Deprecated, runs@PreMatchingon every controller request, and has no test coverage — an unverifiable live behavior change that should not ride along with mechanical deletions. Deserves its own PR.Follow-up work
Staged endpoint deletions. Three endpoints had their callers migrated in the PRs above but were intentionally left in place, so that an N-1 component still works against an N component during a rolling upgrade. They can be deleted once these PRs have shipped in a release:
GET /table/{tableName}/size(singular) — caller repointed in Remove APIs deprecated in pre-1.0 releases (0.1.0-0.12.0 era) #19139GET /segments/{tableName}/select— caller repointed in Remove APIs deprecated in 1.0.0 #19140Remaining release lines. The audit identified further removable APIs deprecated in 1.4.0, 1.5.0, and within the unreleased 1.6.0 cycle. Following the same one-PR-per-release pattern:
InstancePlanMakerImplV2legacy constant block, whose own comment set a "remove after 1.4" deadline that has now passed twice, andPinotHelixResourceManager.getSchemaForTableConfig, which rode along in Remove APIs deprecated in 1.0.0 #19140 with its true vintage noted)segment.local.functionforwarding shims and theV1Constantskeys that need a stop-writing-then-stop-reading two-stepNew deprecations to add. The audit also found legacy surface that is not yet marked at all. Worth its own discussion rather than bundling here — notably the
HttpClientauth-less overloads (documented as deprecated but never annotated), the silent old→new config-key fallbacks that log nothing when the legacy key is used, and the pre-fieldConfigListindex-declaration fields onIndexingConfig.Bugs found in passing
These exist on master today and are unrelated to the PRs above. Filing separately, listed here for traceability:
SegmentAdminClientDELETE path: an empty/null segment list omits thesegmentsquery parameter, which the controller interprets as "delete all segments of the table"SegmentAdminClientDELETE path: multi-segment deletes comma-join into a single parameter value that never matches the controller's repeated@QueryParam("segments")binding, so they silently no-opTenantAdminClient.changeTenantStateissuesPUT /tenants/{tenantName}/metadata, but that path declares only@GETand@POST— it can only ever return 405