From 52a28ac0db11cc902b469fed1aed10256ba94825 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Thu, 5 Dec 2024 16:28:08 -0500 Subject: [PATCH] DO NOT COMMIT Cherry-pick CNDB-7343: Fix RequestFailureReason reason codes according to Appache and avoid future conflicts by assigning new codes to higher numbers CNDB-7343: Fix fromCode method and add a unit test CNDB-7343: Update forException method for IndexNotAvailable CNDB-7343: Make forException method more resiliant against partial updates --- .../exceptions/RequestFailureReason.java | 48 ++++++------- .../exceptions/RequestFailureReasonTest.java | 70 +++++++++++++++++++ 2 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java diff --git a/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java b/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java index b7ac5815f29b..c2f95910355e 100644 --- a/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java +++ b/src/java/org/apache/cassandra/exceptions/RequestFailureReason.java @@ -18,10 +18,13 @@ package org.apache.cassandra.exceptions; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import com.google.common.primitives.Ints; import org.apache.cassandra.db.filter.TombstoneOverwhelmingException; +import org.apache.cassandra.index.IndexNotAvailableException; import org.apache.cassandra.index.sai.utils.AbortedOperationException; import org.apache.cassandra.io.IVersionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; @@ -37,10 +40,12 @@ public enum RequestFailureReason READ_TOO_MANY_TOMBSTONES (1), TIMEOUT (2), INCOMPATIBLE_SCHEMA (3), - INDEX_NOT_AVAILABLE (4), - UNKNOWN_COLUMN (5), - UNKNOWN_TABLE (6), - REMOTE_STORAGE_FAILURE (7); + INDEX_NOT_AVAILABLE (6), // We match it to Apache Cassandra's INDEX_NOT_AVAILABLE code introduced in 5.0 + // The following codes are not present in Apache Cassandra's RequestFailureReason + // We should add new codes in HCD (which do not exist in Apache Cassandra) only with big numbers, to avoid conflicts + UNKNOWN_COLUMN (500), + UNKNOWN_TABLE (501), + REMOTE_STORAGE_FAILURE (502); public static final Serializer serializer = new Serializer(); @@ -58,26 +63,28 @@ public int codeForNativeProtocol() return code; } - private static final RequestFailureReason[] codeToReasonMap; + private static final Map codeToReasonMap = new HashMap<>(); + private static final Map, RequestFailureReason> exceptionToReasonMap = new HashMap<>(); static { RequestFailureReason[] reasons = values(); - int max = -1; - for (RequestFailureReason r : reasons) - max = max(r.code, max); - - RequestFailureReason[] codeMap = new RequestFailureReason[max + 1]; - for (RequestFailureReason reason : reasons) { - if (codeMap[reason.code] != null) + if (codeToReasonMap.put(reason.code, reason) != null) throw new RuntimeException("Two RequestFailureReason-s that map to the same code: " + reason.code); - codeMap[reason.code] = reason; } - codeToReasonMap = codeMap; + exceptionToReasonMap.put(TombstoneOverwhelmingException.class, READ_TOO_MANY_TOMBSTONES); + exceptionToReasonMap.put(IncompatibleSchemaException.class, INCOMPATIBLE_SCHEMA); + exceptionToReasonMap.put(AbortedOperationException.class, TIMEOUT); + exceptionToReasonMap.put(IndexNotAvailableException.class, INDEX_NOT_AVAILABLE); + exceptionToReasonMap.put(UnknownColumnException.class, UNKNOWN_COLUMN); + exceptionToReasonMap.put(UnknownTableException.class, UNKNOWN_TABLE); + + if (exceptionToReasonMap.size() != reasons.length-2) + throw new RuntimeException("A new RequestFailureReasons was probably added and you may need to update the exceptionToReasonMap"); } public static RequestFailureReason fromCode(int code) @@ -86,21 +93,12 @@ public static RequestFailureReason fromCode(int code) throw new IllegalArgumentException("RequestFailureReason code must be non-negative (got " + code + ')'); // be forgiving and return UNKNOWN if we aren't aware of the code - for forward compatibility - return code < codeToReasonMap.length ? codeToReasonMap[code] : UNKNOWN; + return codeToReasonMap.getOrDefault(code, UNKNOWN); } public static RequestFailureReason forException(Throwable t) { - if (t instanceof TombstoneOverwhelmingException) - return READ_TOO_MANY_TOMBSTONES; - - if (t instanceof IncompatibleSchemaException) - return INCOMPATIBLE_SCHEMA; - - if (t instanceof AbortedOperationException) - return TIMEOUT; - - return UNKNOWN; + return exceptionToReasonMap.getOrDefault(t.getClass(), UNKNOWN); } public static final class Serializer implements IVersionedSerializer diff --git a/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java b/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java new file mode 100644 index 000000000000..1de16b5de5f2 --- /dev/null +++ b/test/unit/org/apache/cassandra/exceptions/RequestFailureReasonTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.exceptions; +import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +public class RequestFailureReasonTest +{ + private static final RequestFailureReason[] REASONS = RequestFailureReason.values(); + private static final Object[][] EXPECTED_VALUES = + { + { 0, "UNKNOWN" }, + { 1, "READ_TOO_MANY_TOMBSTONES" }, + { 2, "TIMEOUT" }, + { 3, "INCOMPATIBLE_SCHEMA" }, + { 6, "INDEX_NOT_AVAILABLE" }, + { 500, "UNKNOWN_COLUMN" }, + { 501, "UNKNOWN_TABLE" }, + { 502, "REMOTE_STORAGE_FAILURE" } + }; + @Test + public void testEnumCodesAndNames() + { + for (int i = 0; i < REASONS.length; i++) + { + assertEquals("RequestFailureReason code mismatch for " + + REASONS[i].name(), EXPECTED_VALUES[i][0], REASONS[i].code); + assertEquals("RequestFailureReason name mismatch for code " + + REASONS[i].code, EXPECTED_VALUES[i][1], REASONS[i].name()); + } + assertEquals("Number of RequestFailureReason enum constants has changed. Update the test.", + EXPECTED_VALUES.length, REASONS.length); + } + + @Test + public void testFromCode() + { + // Test valid codes + assertEquals(RequestFailureReason.UNKNOWN, RequestFailureReason.fromCode(0)); + assertEquals(RequestFailureReason.READ_TOO_MANY_TOMBSTONES, RequestFailureReason.fromCode(1)); + assertEquals(RequestFailureReason.TIMEOUT, RequestFailureReason.fromCode(2)); + assertEquals(RequestFailureReason.INCOMPATIBLE_SCHEMA, RequestFailureReason.fromCode(3)); + assertEquals(RequestFailureReason.INDEX_NOT_AVAILABLE, RequestFailureReason.fromCode(6)); + assertEquals(RequestFailureReason.UNKNOWN_COLUMN, RequestFailureReason.fromCode(500)); + assertEquals(RequestFailureReason.UNKNOWN_TABLE, RequestFailureReason.fromCode(501)); + assertEquals(RequestFailureReason.REMOTE_STORAGE_FAILURE, RequestFailureReason.fromCode(502)); + + // Test invalid codes + assertEquals(RequestFailureReason.UNKNOWN, RequestFailureReason.fromCode(200)); + assertEquals(RequestFailureReason.UNKNOWN, RequestFailureReason.fromCode(999)); + assertThrows(IllegalArgumentException.class, () -> RequestFailureReason.fromCode(-1)); + } +}