You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by mc...@apache.org on 2020/04/06 16:27:06 UTC
[cassandra] branch trunk updated: Fix RepairCoordinator test
failures,
after clobbering jvm-dtest refactoring (CASSANDRA-15650) and modifying
classes no longer in the project
This is an automated email from the ASF dual-hosted git repository.
mck pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new a104b06 Fix RepairCoordinator test failures, after clobbering jvm-dtest refactoring (CASSANDRA-15650) and modifying classes no longer in the project
a104b06 is described below
commit a104b06d4aea2f2cd3d48bdbe38410284f236428
Author: David Capwell <dc...@gmail.com>
AuthorDate: Thu Apr 2 10:58:43 2020 -0700
Fix RepairCoordinator test failures, after clobbering jvm-dtest refactoring (CASSANDRA-15650) and modifying classes no longer in the project
patch by David Capwell; reviewed by Benjamin Lerer, Alex Petrov for CASSANDRA-15684
---
.../cassandra/distributed/api/LongTokenRange.java | 38 ----
.../cassandra/distributed/api/NodeToolResult.java | 218 ---------------------
.../cassandra/distributed/api/QueryResult.java | 139 -------------
.../org/apache/cassandra/distributed/api/Row.java | 119 -----------
.../distributed/test/RepairCoordinatorFast.java | 8 +-
5 files changed, 6 insertions(+), 516 deletions(-)
diff --git a/test/distributed/org/apache/cassandra/distributed/api/LongTokenRange.java b/test/distributed/org/apache/cassandra/distributed/api/LongTokenRange.java
deleted file mode 100644
index 06327e8..0000000
--- a/test/distributed/org/apache/cassandra/distributed/api/LongTokenRange.java
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * 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.distributed.api;
-
-import java.io.Serializable;
-
-public final class LongTokenRange implements Serializable
-{
- public final long minExclusive;
- public final long maxInclusive;
-
- public LongTokenRange(long minExclusive, long maxInclusive)
- {
- this.minExclusive = minExclusive;
- this.maxInclusive = maxInclusive;
- }
-
- public String toString()
- {
- return "(" + minExclusive + "," + maxInclusive + "]";
- }
-}
diff --git a/test/distributed/org/apache/cassandra/distributed/api/NodeToolResult.java b/test/distributed/org/apache/cassandra/distributed/api/NodeToolResult.java
deleted file mode 100644
index 8f33ae5..0000000
--- a/test/distributed/org/apache/cassandra/distributed/api/NodeToolResult.java
+++ /dev/null
@@ -1,218 +0,0 @@
-/*
- * 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.distributed.api;
-
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import javax.management.Notification;
-
-import com.google.common.base.Throwables;
-import org.junit.Assert;
-
-public class NodeToolResult
-{
- private final String[] commandAndArgs;
- private final int rc;
- private final List<Notification> notifications;
- private final Throwable error;
-
- public NodeToolResult(String[] commandAndArgs, int rc, List<Notification> notifications, Throwable error)
- {
- this.commandAndArgs = commandAndArgs;
- this.rc = rc;
- this.notifications = notifications;
- this.error = error;
- }
-
- public String[] getCommandAndArgs()
- {
- return commandAndArgs;
- }
-
- public int getRc()
- {
- return rc;
- }
-
- public List<Notification> getNotifications()
- {
- return notifications;
- }
-
- public Throwable getError()
- {
- return error;
- }
-
- public Asserts asserts()
- {
- return new Asserts();
- }
-
- public final class Asserts {
- public Asserts success() {
- if (rc != 0)
- fail("was not successful");
- return this;
- }
-
- public Asserts failure() {
- if (rc == 0)
- fail("was successful but not expected to be");
- return this;
- }
-
- public Asserts errorContains(String... messages) {
- Assert.assertNotEquals("no error messages defined to check against", 0, messages.length);
- Assert.assertNotNull("No exception was found but expected one", error);
- if (!Stream.of(messages).anyMatch(msg -> error.getMessage().contains(msg)))
- fail("Error message '" + error.getMessage() + "' does not contain any of " + Arrays.toString(messages));
- return this;
- }
-
- public Asserts notificationContains(String msg) {
- Assert.assertNotNull("notifications not defined", notifications);
- Assert.assertFalse("notifications not defined", notifications.isEmpty());
- for (Notification n : notifications) {
- if (n.getMessage().contains(msg)) {
- return this;
- }
- }
- fail("Unable to locate message " + msg + " in notifications: " + NodeToolResult.toString(notifications));
- return this; // unreachable
- }
-
- public Asserts notificationContains(ProgressEventType type, String msg) {
- int userType = type.ordinal();
- Assert.assertNotNull("notifications not defined", notifications);
- Assert.assertFalse("notifications not defined", notifications.isEmpty());
- for (Notification n : notifications) {
- if (notificationType(n) == userType) {
- if (n.getMessage().contains(msg)) {
- return this;
- }
- }
- }
- fail("Unable to locate message '" + msg + "' in notifications: " + NodeToolResult.toString(notifications));
- return this; // unreachable
- }
-
- private void fail(String message)
- {
- StringBuilder sb = new StringBuilder();
- sb.append("nodetool command ").append(Arrays.toString(commandAndArgs)).append(" ").append(message).append("\n");
- sb.append("Notifications:\n");
- for (Notification n : notifications)
- sb.append(NodeToolResult.toString(n)).append("\n");
- if (error != null)
- sb.append("Error:\n").append(Throwables.getStackTraceAsString(error)).append("\n");
- throw new AssertionError(sb.toString());
- }
- }
-
- private static String toString(Collection<Notification> notifications)
- {
- return notifications.stream().map(NodeToolResult::toString).collect(Collectors.joining(", "));
- }
-
- private static String toString(Notification notification)
- {
- ProgressEventType type = ProgressEventType.values()[notificationType(notification)];
- String msg = notification.getMessage();
- Object src = notification.getSource();
- return "Notification{" +
- "type=" + type +
- ", src=" + src +
- ", message=" + msg +
- "}";
- }
-
- private static int notificationType(Notification n)
- {
- return ((Map<String, Integer>) n.getUserData()).get("type").intValue();
- }
-
- public String toString()
- {
- return "NodeToolResult{" +
- "commandAndArgs=" + Arrays.toString(commandAndArgs) +
- ", rc=" + rc +
- ", notifications=[" + notifications.stream().map(n -> ProgressEventType.values()[notificationType(n)].name()).collect(Collectors.joining(", ")) + "]" +
- ", error=" + error +
- '}';
- }
-
- /**
- * Progress event type.
- *
- * <p>
- * Progress starts by emitting {@link #START}, followed by emitting zero or more {@link #PROGRESS} events,
- * then it emits either one of {@link #ERROR}/{@link #ABORT}/{@link #SUCCESS}.
- * Progress indicates its completion by emitting {@link #COMPLETE} at the end of process.
- * </p>
- * <p>
- * {@link #NOTIFICATION} event type is used to just notify message without progress.
- * </p>
- */
- public enum ProgressEventType
- {
- /**
- * Fired first when progress starts.
- * Happens only once.
- */
- START,
-
- /**
- * Fire when progress happens.
- * This can be zero or more time after START.
- */
- PROGRESS,
-
- /**
- * When observing process completes with error, this is sent once before COMPLETE.
- */
- ERROR,
-
- /**
- * When observing process is aborted by user, this is sent once before COMPLETE.
- */
- ABORT,
-
- /**
- * When observing process completes successfully, this is sent once before COMPLETE.
- */
- SUCCESS,
-
- /**
- * Fire when progress complete.
- * This is fired once, after ERROR/ABORT/SUCCESS is fired.
- * After this, no more ProgressEvent should be fired for the same event.
- */
- COMPLETE,
-
- /**
- * Used when sending message without progress.
- */
- NOTIFICATION
- }
-}
diff --git a/test/distributed/org/apache/cassandra/distributed/api/QueryResult.java b/test/distributed/org/apache/cassandra/distributed/api/QueryResult.java
deleted file mode 100644
index dcdfa14..0000000
--- a/test/distributed/org/apache/cassandra/distributed/api/QueryResult.java
+++ /dev/null
@@ -1,139 +0,0 @@
-/*
- * 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.distributed.api;
-
-import java.util.Iterator;
-import java.util.NoSuchElementException;
-import java.util.Objects;
-import java.util.function.Predicate;
-
-/**
- * A table of data representing a complete query result.
- *
- * A <code>QueryResult</code> is different from {@link java.sql.ResultSet} in several key ways:
- *
- * <ul>
- * <li>represents a complete result rather than a cursor</li>
- * <li>returns a {@link Row} to access the current row of data</li>
- * <li>relies on object pooling; {@link #hasNext()} may return the same object just with different data, accessing a
- * {@link Row} from a previous {@link #hasNext()} call has undefined behavior.</li>
- * <li>includes {@link #filter(Predicate)}, this will do client side filtering since Apache Cassandra is more
- * restrictive on server side filtering</li>
- * </ul>
- *
- * <h2>Unsafe patterns</h2>
- *
- * Below are a few unsafe patterns which may lead to unexpected results
- *
- * <code>{@code
- * while (rs.hasNext()) {
- * list.add(rs.next());
- * }
- * }</code>
- *
- * <code>{@code
- * rs.forEach(list::add)
- * }</code>
- *
- * Both cases have the same issue; reference to a row from a previous call to {@link #hasNext()}. Since the same {@link Row}
- * object can be used accross different calls to {@link #hasNext()} this would mean any attempt to access after the fact
- * points to newer data. If this behavior is not desirable and access is needed between calls, then {@link Row#copy()}
- * should be used; this will clone the {@link Row} and return a new object pointing to the same data.
- */
-public class QueryResult implements Iterator<Row>
-{
- public static final QueryResult EMPTY = new QueryResult(new String[0], null);
-
- private final String[] names;
- private final Object[][] results;
- private final Predicate<Row> filter;
- private final Row row;
- private int offset = -1;
-
- public QueryResult(String[] names, Object[][] results)
- {
- this.names = Objects.requireNonNull(names, "names");
- this.results = results;
- this.row = new Row(names);
- this.filter = ignore -> true;
- }
-
- private QueryResult(String[] names, Object[][] results, Predicate<Row> filter, int offset)
- {
- this.names = names;
- this.results = results;
- this.filter = filter;
- this.offset = offset;
- this.row = new Row(names);
- }
-
- public String[] getNames()
- {
- return names;
- }
-
- public boolean isEmpty()
- {
- return results.length == 0;
- }
-
- public int size()
- {
- return results.length;
- }
-
- public QueryResult filter(Predicate<Row> fn)
- {
- return new QueryResult(names, results, filter.and(fn), offset);
- }
-
- /**
- * Get all rows as a 2d array. Any calls to {@link #filter(Predicate)} will be ignored and the array returned will
- * be the full set from the query.
- */
- public Object[][] toObjectArrays()
- {
- return results;
- }
-
- @Override
- public boolean hasNext()
- {
- if (results == null)
- return false;
- while ((offset += 1) < results.length)
- {
- row.setResults(results[offset]);
- if (filter.test(row))
- {
- return true;
- }
- }
- row.setResults(null);
- return false;
- }
-
- @Override
- public Row next()
- {
- if (offset < 0 || offset >= results.length)
- throw new NoSuchElementException();
- return row;
- }
-}
diff --git a/test/distributed/org/apache/cassandra/distributed/api/Row.java b/test/distributed/org/apache/cassandra/distributed/api/Row.java
deleted file mode 100644
index 43fa6d9..0000000
--- a/test/distributed/org/apache/cassandra/distributed/api/Row.java
+++ /dev/null
@@ -1,119 +0,0 @@
-/*
- * 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.distributed.api;
-
-import java.util.Arrays;
-import java.util.Date;
-import java.util.NoSuchElementException;
-import java.util.Objects;
-import java.util.Set;
-import java.util.UUID;
-import javax.annotation.Nullable;
-
-import com.carrotsearch.hppc.ObjectIntHashMap;
-import com.carrotsearch.hppc.ObjectIntMap;
-
-/**
- * Data representing a single row in a query result.
- *
- * This class is mutable from the parent {@link QueryResult} and can have the row it points to changed between calls
- * to {@link QueryResult#hasNext()}, for this reason it is unsafe to hold reference to this class after that call;
- * to get around this, a call to {@link #copy()} will return a new object pointing to the same row.
- */
-public class Row
-{
- private final ObjectIntMap<String> nameIndex;
- @Nullable private Object[] results; // mutable to avoid allocations in loops
-
- public Row(String[] names)
- {
- Objects.requireNonNull(names, "names");
- this.nameIndex = new ObjectIntHashMap<>(names.length);
- for (int i = 0; i < names.length; i++) {
- nameIndex.put(names[i], i);
- }
- }
-
- private Row(ObjectIntMap<String> nameIndex)
- {
- this.nameIndex = nameIndex;
- }
-
- void setResults(@Nullable Object[] results)
- {
- this.results = results;
- }
-
- /**
- * Creates a copy of the current row; can be used past calls to {@link QueryResult#hasNext()}.
- */
- public Row copy() {
- Row copy = new Row(nameIndex);
- copy.setResults(results);
- return copy;
- }
-
- public <T> T get(String name)
- {
- checkAccess();
- int idx = findIndex(name);
- if (idx == -1)
- return null;
- return (T) results[idx];
- }
-
- public String getString(String name)
- {
- return get(name);
- }
-
- public UUID getUUID(String name)
- {
- return get(name);
- }
-
- public Date getTimestamp(String name)
- {
- return get(name);
- }
-
- public <T> Set<T> getSet(String name)
- {
- return get(name);
- }
-
- public String toString()
- {
- return "Row{" +
- "names=" + nameIndex.keys() +
- ", results=" + Arrays.toString(results) +
- '}';
- }
-
- private void checkAccess()
- {
- if (results == null)
- throw new NoSuchElementException();
- }
-
- private int findIndex(String name)
- {
- return nameIndex.getOrDefault(name, -1);
- }
-}
diff --git a/test/distributed/org/apache/cassandra/distributed/test/RepairCoordinatorFast.java b/test/distributed/org/apache/cassandra/distributed/test/RepairCoordinatorFast.java
index 8d26b2e..0e156da 100644
--- a/test/distributed/org/apache/cassandra/distributed/test/RepairCoordinatorFast.java
+++ b/test/distributed/org/apache/cassandra/distributed/test/RepairCoordinatorFast.java
@@ -369,13 +369,17 @@ public abstract class RepairCoordinatorFast extends RepairCoordinatorBase
long repairExceptions = getRepairExceptions(CLUSTER, 1);
NodeToolResult result = repair(1, KEYSPACE, table);
result.asserts()
- .failure()
+ .failure();
// Right now coordination doesn't propgate the first exception, so we only know "there exists a issue".
// With notifications on nodetool will see the error then complete, so the cmd state (what nodetool
// polls on) is ignored. With notifications off or dropped, the poll await fails and queries cmd
// state, and that will have the below error.
// NOTE: this isn't desireable, would be good to propgate
- .errorContains("Could not create snapshot", "Some repair failed");
+ // TODO replace with errorContainsAny once dtest api updated
+ Throwable error = result.getError();
+ Assert.assertNotNull("Error was null", error);
+ if (!(error.getMessage().contains("Could not create snapshot") || error.getMessage().contains("Some repair failed")))
+ throw new AssertionError("Unexpected error, expected to contain 'Could not create snapshot' or 'Some repair failed'", error);
if (withNotifications)
{
result.asserts()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org