[Controller] Add gRPC support for deleteStore API #2425
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.
Problem Statement
As part of the controller gRPC migration, the
deleteStoreAPI needs to be available via gRPC while maintaining the existing HTTP endpoint during the migration period. This follows the same pattern established by other controller API migrations, where business logic is abstracted so both HTTP and gRPC can share the same implementation.Solution
Added gRPC support for the
deleteStoreAPI following the established migration pattern:StoreGrpcService.proto): AddeddeleteStoreRPC method withDeleteStoreGrpcRequestandDeleteStoreGrpcResponsemessagesStoreRequestHandler.java): AddeddeleteStore()method that validates parameters and callsadmin.deleteStore(), returning a gRPC responseStoreGrpcServiceImpl.java): AddeddeleteStore()override with ACL checks (matching the HTTP endpoint behavior)StoresRoutes.java): Updated to convert HTTP requests to gRPC format and delegate to the shared handlerAdminSparkServer.java): No changes needed - handler already passed to StoresRoutes constructorThe implementation reuses the existing business logic in
Admin.deleteStore(), ensuring consistency between HTTP and gRPC endpoints.Code changes
Concurrency-Specific Checks
Both reviewer and PR author to verify
synchronized,RWLock) are used where needed. (Uses existing AdminCommandExecutionTracker synchronization pattern)ConcurrentHashMap,CopyOnWriteArrayList). (N/A - no collections introduced)handleRequest()utility for consistent error handling)How was this PR tested?
testDeleteStoreReturnsSuccessfulResponse- Verifies successful response with execution IDtestDeleteStoreReturnsSuccessfulResponseWithoutExecutionId- Verifies response without execution IDtestDeleteStoreWithAbortMigrationCleanup- Verifies abort migration cleanup flagtestDeleteStoreReturnsErrorResponse- Verifies error handlingtestDeleteStoreReturnsPermissionDenied- Verifies ACL enforcementtestDeleteStoreReturnsBadRequestForInvalidArgument- Verifies invalid argument handlingtestDeleteStore(StoresRoutesTest) - Verifies HTTP route integration with handlertestDeleteStoreSuccess- Verifies request handler success pathtestDeleteStoreWithAbortMigrationCleanup- Verifies request handler with flagtestDeleteStoreWithExecutionTracker- Verifies execution ID trackingtestDeleteStoreThrowsException- Verifies exception handlingtestDeleteStoreMissingClusterName- Verifies validationtestDeleteStoreMissingStoreName- Verifies validationtestDeleteStoreGrpcEndpoint- End-to-end test for deleteStore via gRPCtestDeleteStoreOverSecureGrpcChannel- Secure channel ACL enforcement test./gradlew :services:venice-controller:test :services:venice-controller:jacocoTestReportDoes this PR introduce any user-facing or breaking changes?