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 <dcapw...@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-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org