xiangfu0 opened a new pull request, #18755:
URL: https://github.com/apache/pinot/pull/18755
## Description
Fixes a class of correctness bugs in `PinotAdminClient` and completes the
`ControllerRequestClient` → `PinotAdminClient` refactor (#15537).
`PinotAdminClient`'s admin sub-clients had diverged from the controller REST
contract. The unit tests are mock-transport tests that asserted the *client's
own assumed* JSON shapes, so the bugs passed CI while failing against a live
controller. Each fix below was verified against the actual controller
`*RestletResource` class.
### Contract fixes (`pinot-java-client`)
**Wrong response shape (threw / NPE'd / returned null):**
- `SchemaAdminClient.listSchemaNames` read `"schemas"`; `GET /schemas`
returns a bare array.
- `InstanceAdminClient.listLiveInstances` read `"liveInstances"`;
`/liveinstances` returns `{"instances":[…]}`.
- `TableAdminClient.cancelRebalance` read `"jobIds"`; `DELETE
/tables/{t}/rebalance` returns a bare array.
- `SegmentAdminClient.getZookeeperMetadata` read `"zkMetadata"`; the
endpoint returns a bare map.
- `TenantAdminClient.listTenantsAsync` read `"tenants"`; `GET /tenants`
returns `{SERVER_TENANTS, BROKER_TENANTS}`.
- Every `TaskAdminClient` endpoint resumes a bare value
(`Set`/`TaskState`/`int`/`Map`), not a wrapper object.
**Wrong query-param names (silently ignored by the controller):**
- Tenant rebalance: `includeTables`/`excludeTables` (were
`includeTableTypes`/`excludeTableTypes`/`rebalanceMode`).
- Task counts: `table`; task debug: `tableName` (were `tableNameWithType`).
- Segment select: `startTimestamp`/`endTimestamp` (were `…Ms`).
- Table rebalance: `downtime`/`minAvailableReplicas`.
**Missing required body:**
- Tenant rebalance now sends a `TenantRebalanceConfig` body; instance
updateTags validation now sends a `List<InstanceTagUpdateRequest>` body.
**Refactor:** consolidated bare string-array parsing into
`PinotAdminTransport.parseStringArrayNode` (strict for synchronous callers,
lenient wrapper for async).
### `ControllerRequestClient` retirement (#15537)
- Migrated the last `ControllerRequestClient` usage
(`BaseMultiClusterIntegrationTest`) to the existing `addSchemaToCluster` helper.
- Deleted the `ControllerRequestClient` class.
- Renamed `getControllerRequestClientHeaders` → `getAdminClientHeaders`.
## Testing
- Corrected the mocks that asserted the wrong shapes; added regression tests
for each fixed endpoint plus the new parser edge cases —
`PinotAdminClientTest`: **36 passing**.
- Full-repo `test-compile` passes (validates the class deletion and that no
caller relied on the changed `PinotAdminClient` signatures).
- spotless / checkstyle / license: clean.
## Backward-incompatible changes
None affecting released APIs. `PinotAdminClient` is new in this release
line; two of its unreleased convenience overloads (`rebalanceTable`,
`rebalanceTenant`) dropped parameters that mapped to nothing on the controller.
The `ControllerRequestClient` test utility is removed and replaced by
`PinotAdminClient`.
## Release notes
Completes #15537.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
