This is an automated email from the ASF dual-hosted git repository. kturner pushed a commit to branch elasticity in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push: new e98f15b5fd fixes deleting tablet suspension (#4575) e98f15b5fd is described below commit e98f15b5fd282ee60d2d695531ee16b89014d459 Author: Keith Turner <ktur...@apache.org> AuthorDate: Mon May 20 16:17:45 2024 -0400 fixes deleting tablet suspension (#4575) Tablet suspension was failing to delete becasue the value set on the conditional mutation did not match the format of what was stored in the metadata table. Fixed this issue and added some unit test. --- .../org/apache/accumulo/core/data/Condition.java | 15 ++++++ .../accumulo/core/metadata/SuspendingTServer.java | 4 ++ .../core/metadata/SuspendingTServerTest.java | 63 ++++++++++++++++++++++ .../metadata/ConditionalTabletMutatorImpl.java | 3 +- 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/data/Condition.java b/core/src/main/java/org/apache/accumulo/core/data/Condition.java index deadc0e069..dab5ee24e5 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Condition.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Condition.java @@ -209,6 +209,21 @@ public class Condition { return this; } + /** + * This method sets the expected value of a column. In order for the condition to pass the column + * must exist and have this value. If a value is not set, then the column must be absent for the + * condition to pass. See {@link #setValue(byte[])}. + * + * @param value value + * @return this condition + * @throws IllegalArgumentException if value is null + * @since 4.0.0 + */ + public Condition setValue(Value value) { + checkArgument(value != null, "value is null"); + return setValue(value.get()); + } + /** * Gets the value of this condition. * diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/SuspendingTServer.java b/core/src/main/java/org/apache/accumulo/core/metadata/SuspendingTServer.java index e481369a21..e45635fc80 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/SuspendingTServer.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/SuspendingTServer.java @@ -49,6 +49,10 @@ public class SuspendingTServer { return new Value(tServer.getHostPort() + "|" + suspensionTime.getMillis()); } + public Value toValue() { + return new Value(server + "|" + suspensionTime.getMillis()); + } + @Override public boolean equals(Object rhsObject) { if (!(rhsObject instanceof SuspendingTServer)) { diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/SuspendingTServerTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/SuspendingTServerTest.java new file mode 100644 index 0000000000..7826915ed5 --- /dev/null +++ b/core/src/test/java/org/apache/accumulo/core/metadata/SuspendingTServerTest.java @@ -0,0 +1,63 @@ +/* + * 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 + * + * https://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.accumulo.core.metadata; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.util.time.SteadyTime; +import org.junit.jupiter.api.Test; + +import com.google.common.net.HostAndPort; + +public class SuspendingTServerTest { + @Test + public void testToFromValue() { + SteadyTime suspensionTime = SteadyTime.from(System.currentTimeMillis(), TimeUnit.MILLISECONDS); + TServerInstance ser1 = new TServerInstance(HostAndPort.fromParts("server1", 8555), "s001"); + + var val1 = SuspendingTServer.toValue(ser1, suspensionTime); + var st1 = SuspendingTServer.fromValue(val1); + assertEquals(HostAndPort.fromParts("server1", 8555), st1.server); + assertEquals(suspensionTime, st1.suspensionTime); + assertEquals(val1, st1.toValue()); + var st2 = new SuspendingTServer(HostAndPort.fromParts("server1", 8555), suspensionTime); + assertEquals(st1, st2); + assertEquals(st1.hashCode(), st2.hashCode()); + assertEquals(st1.toString(), st2.toString()); + assertEquals(st1.toValue(), st2.toValue()); + + // Create three SuspendingTServer objs that differ in one field. Ensure each field is considered + // in equality checks. + var st3 = new SuspendingTServer(HostAndPort.fromParts("server2", 8555), suspensionTime); + var st4 = new SuspendingTServer(HostAndPort.fromParts("server1", 9555), suspensionTime); + SteadyTime suspensionTime2 = + SteadyTime.from(System.currentTimeMillis() + 100, TimeUnit.MILLISECONDS); + var st5 = new SuspendingTServer(HostAndPort.fromParts("server1", 8555), suspensionTime2); + for (var stne : List.of(st3, st4, st5)) { + assertNotEquals(st1, stne); + assertNotEquals(st1.toValue(), stne.toValue()); + assertNotEquals(st1.toString(), stne.toString()); + assertNotEquals(st1.hashCode(), stne.hashCode()); + } + } +} diff --git a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java index 85c5e2ebad..7c3d9a5859 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java @@ -255,8 +255,7 @@ public class ConditionalTabletMutatorImpl extends TabletMutatorBase<Ample.Condit Condition c = new Condition(SUSPEND_COLUMN.getColumnFamily(), SUSPEND_COLUMN.getColumnQualifier()); if (tabletMetadata.getSuspend() != null) { - c.setValue(tabletMetadata.getSuspend().server + "|" - + tabletMetadata.getSuspend().suspensionTime); + c.setValue(tabletMetadata.getSuspend().toValue()); } mutation.addCondition(c); }