Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - do not review or commit! Fix RequestFailureReason reason codes according to Apache and avoid f… #1457

Draft
wants to merge 1 commit into
base: main-5.0
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 26 additions & 40 deletions src/java/org/apache/cassandra/exceptions/RequestFailureReason.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,41 @@
package org.apache.cassandra.exceptions;

import java.io.IOException;
import java.util.function.ToIntFunction;

import org.apache.cassandra.db.filter.TombstoneOverwhelmingException;
import org.apache.cassandra.index.sai.utils.AbortedOperationException;
import org.apache.cassandra.io.IVersionedSerializer;
import org.apache.cassandra.io.util.DataInputPlus;
import org.apache.cassandra.io.util.DataOutputPlus;
import org.apache.cassandra.net.MessagingService;
import org.apache.cassandra.utils.vint.VIntCoding;

import static java.lang.Math.max;
import static org.apache.cassandra.net.MessagingService.VERSION_40;

public enum RequestFailureReason
{
UNKNOWN (0, 0),
READ_TOO_MANY_TOMBSTONES (1, 1),
TIMEOUT (2, 2),
INCOMPATIBLE_SCHEMA (3, 3),
READ_SIZE (4, 4),
NODE_DOWN (5, 8),
INDEX_NOT_AVAILABLE (6, 9),
READ_TOO_MANY_INDEXES (7, 10),
UNKNOWN_COLUMN (8, 5),
UNKNOWN_TABLE (9, 6),
REMOTE_STORAGE_FAILURE (10, 7),
UNKNOWN (0),
READ_TOO_MANY_TOMBSTONES (1),
TIMEOUT (2),
INCOMPATIBLE_SCHEMA (3),
READ_SIZE (4),
NODE_DOWN (5),
INDEX_NOT_AVAILABLE (6),
READ_TOO_MANY_INDEXES (7),
// 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();

public final int code;
public final int ccCode;

RequestFailureReason(int code, int ccCode)
RequestFailureReason(int code)
{
this.code = code;
this.ccCode = ccCode;
}

public int codeForNativeProtocol()
Expand All @@ -64,41 +62,35 @@ public int codeForNativeProtocol()
return code;
}

private static RequestFailureReason[] makeCodeMapping(ToIntFunction<RequestFailureReason> mappingFunction)
private static final RequestFailureReason[] codeToReasonMap;

static
{
RequestFailureReason[] reasons = values();

int max = -1;
for (RequestFailureReason r : reasons)
max = max(mappingFunction.applyAsInt(r), max);
max = max(r.code, max);

RequestFailureReason[] codeMap = new RequestFailureReason[max + 1];

for (RequestFailureReason reason : reasons)
{
if (codeMap[mappingFunction.applyAsInt(reason)] != null)
if (codeMap[reason.code] != null)
throw new RuntimeException("Two RequestFailureReason-s that map to the same code: " + reason.code);
codeMap[mappingFunction.applyAsInt(reason)] = reason;
codeMap[reason.code] = reason;
}

return codeMap;
codeToReasonMap = codeMap;
}

private static final RequestFailureReason[] codeToReasonMap = makeCodeMapping(r -> r.code);
private static final RequestFailureReason[] ccCodeToReasonMap = makeCodeMapping(r -> r.ccCode);

public static RequestFailureReason fromCode(int code, int version)
public static RequestFailureReason fromCode(int code)
{
assert version >= VERSION_40;

if (code < 0)
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
if (version >= MessagingService.VERSION_SG_10)
return code < ccCodeToReasonMap.length ? ccCodeToReasonMap[code] : UNKNOWN;
else
return code < codeToReasonMap.length ? codeToReasonMap[code] : UNKNOWN;
return code < codeToReasonMap.length ? codeToReasonMap[code] : UNKNOWN;
}

public static RequestFailureReason forException(Throwable t)
Expand All @@ -124,25 +116,19 @@ private Serializer()
public void serialize(RequestFailureReason reason, DataOutputPlus out, int version) throws IOException
{
assert version >= VERSION_40;
if (version >= MessagingService.VERSION_SG_10)
out.writeUnsignedVInt32(reason.ccCode);
else
out.writeUnsignedVInt32(reason.code);
out.writeUnsignedVInt32(reason.code);
}

public RequestFailureReason deserialize(DataInputPlus in, int version) throws IOException
{
assert version >= VERSION_40;
return fromCode(in.readUnsignedVInt32(), version);
return fromCode(in.readUnsignedVInt32());
}

public long serializedSize(RequestFailureReason reason, int version)
{
assert version >= VERSION_40;
if (version >= MessagingService.VERSION_SG_10)
return VIntCoding.computeVIntSize(reason.ccCode);
else
return VIntCoding.computeVIntSize(reason.code);
return VIntCoding.computeVIntSize(reason.code);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.apache.cassandra.db.WriteType;
import org.apache.cassandra.exceptions.*;
import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.net.MessagingService;
import org.apache.cassandra.transport.*;
import org.apache.cassandra.utils.MD5Digest;

Expand Down Expand Up @@ -98,7 +97,7 @@ public ErrorMessage decode(ByteBuf body, ProtocolVersion version)
for (int i = 0; i < failure; i++)
{
InetAddress endpoint = CBUtil.readInetAddr(body);
RequestFailureReason failureReason = RequestFailureReason.fromCode(body.readUnsignedShort(), MessagingService.current_version); // TODO make sure this is ok - we use different code for CC and different for OSS
RequestFailureReason failureReason = RequestFailureReason.fromCode(body.readUnsignedShort());
builder.put(InetAddressAndPort.getByAddress(endpoint), failureReason);
}
failureReasonByEndpoint = builder.build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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;
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" },
{ 4, "READ_SIZE" },
{ 5, "NODE_DOWN" },
{ 6, "INDEX_NOT_AVAILABLE" },
{ 7, "READ_TOO_MANY_INDEXES" },
{ 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);
}
}