Skip to content

Commit

Permalink
[admin-tool] Fixed mishandling of get store response in store migrati…
Browse files Browse the repository at this point in the history
…on (#968)

Store migration's end migration command assumes /store returns null for non-existing store. However,
it actually throws exception instead and is not handled properly which causes confusion for the user.
  • Loading branch information
xunyin8 committed May 1, 2024
1 parent eade77a commit b2301ac
Showing 1 changed file with 16 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import com.linkedin.venice.datarecovery.EstimateDataRecoveryTimeCommand;
import com.linkedin.venice.datarecovery.MonitorCommand;
import com.linkedin.venice.datarecovery.StoreRepushCommand;
import com.linkedin.venice.exceptions.ErrorType;
import com.linkedin.venice.exceptions.VeniceException;
import com.linkedin.venice.helix.HelixAdapterSerializer;
import com.linkedin.venice.helix.HelixSchemaAccessor;
Expand Down Expand Up @@ -2141,7 +2142,14 @@ private static void endMigration(CommandLine cmd) {
}

// Skip original store deletion if it has already been deleted
if (srcControllerClient.getStore(storeName).getStore() != null) {
StoreResponse srcStoreResponse = srcControllerClient.getStore(storeName);
if (srcStoreResponse.isError() && srcStoreResponse.getErrorType() != ErrorType.STORE_NOT_FOUND) {
System.err.println(
"ERROR: failed to check store " + storeName + " existence in original cluster " + srcClusterName
+ " due to error: " + srcStoreResponse.getError());
}

if (srcStoreResponse.getErrorType() != ErrorType.STORE_NOT_FOUND) {
// Delete original store
srcControllerClient
.updateStore(storeName, new UpdateStoreQueryParams().setEnableReads(false).setEnableWrites(false));
Expand All @@ -2157,7 +2165,13 @@ private static void endMigration(CommandLine cmd) {
ChildAwareResponse response = srcControllerClient.listChildControllers(srcClusterName);
Map<String, ControllerClient> srcChildControllerClientMap = getControllerClientMap(srcClusterName, response);
for (Map.Entry<String, ControllerClient> entry: srcChildControllerClientMap.entrySet()) {
if (entry.getValue().getStore(storeName).getStore() != null) {
StoreResponse childSrcStoreResponse = entry.getValue().getStore(storeName);
if (childSrcStoreResponse.isError() && childSrcStoreResponse.getErrorType() != ErrorType.STORE_NOT_FOUND) {
System.err.println(
"ERROR: failed to check store " + storeName + " existence in original cluster " + srcClusterName
+ " in fabric " + entry.getKey() + " due to error: " + childSrcStoreResponse.getError());
}
if (childSrcStoreResponse.getErrorType() != ErrorType.STORE_NOT_FOUND) {
System.err.println(
"ERROR: store " + storeName + " still exists in source cluster " + srcClusterName + " in fabric "
+ entry.getKey() + ". Please try again later.");
Expand Down

0 comments on commit b2301ac

Please sign in to comment.