This is an automated email from the ASF dual-hosted git repository.
domgarguilo 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 1ebe3352a2 Ensure serialized version of SelectedFiles is comparable
(#3575)
1ebe3352a2 is described below
commit 1ebe3352a2db8ef856fbaf5b8257cd6424976893
Author: Dom G <[email protected]>
AuthorDate: Thu Jul 13 14:47:48 2023 -0400
Ensure serialized version of SelectedFiles is comparable (#3575)
* use custom TypeAdapter for serialization of json
* add tests
---------
Co-authored-by: Keith Turner <[email protected]>
---
core/pom.xml | 5 +
.../core/metadata/schema/SelectedFiles.java | 130 +++++++++----
.../core/metadata/schema/SelectedFilesTest.java | 207 +++++++++++++++++++++
3 files changed, 307 insertions(+), 35 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index ebc84c893e..c486923432 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -168,6 +168,11 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<testResources>
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java
index e0863f53ba..73f005199b 100644
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java
+++
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/SelectedFiles.java
@@ -18,10 +18,11 @@
*/
package org.apache.accumulo.core.metadata.schema;
-import static java.util.stream.Collectors.toList;
-import static org.apache.accumulo.core.util.LazySingletons.GSON;
-
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
@@ -29,6 +30,11 @@ import org.apache.accumulo.core.fate.FateTxId;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import com.google.common.base.Preconditions;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
/**
* This class is used to manage the set of files selected for a user
compaction for a tablet.
@@ -36,50 +42,82 @@ import com.google.common.base.Preconditions;
public class SelectedFiles {
private final Set<StoredTabletFile> files;
-
private final boolean initiallySelectedAll;
private final long fateTxId;
- private final String metadataValue;
-
- // This class is used to serialize and deserialize this class using GSon.
Any changes to this
- // class must consider persisted data.
- private static class GSonData {
- String txid;
- boolean selAll;
- List<String> files;
- }
+
+ private String metadataValue;
+
+ private static final Gson GSON = new GsonBuilder()
+ .registerTypeAdapter(SelectedFiles.class, new
SelectedFilesTypeAdapter()).create();
public SelectedFiles(Set<StoredTabletFile> files, boolean
initiallySelectedAll, long fateTxId) {
Preconditions.checkArgument(files != null && !files.isEmpty());
this.files = Set.copyOf(files);
this.initiallySelectedAll = initiallySelectedAll;
this.fateTxId = fateTxId;
-
- GSonData jData = new GSonData();
- // sort to make the serialized version equals when the sets are equal
- jData.files =
-
files.stream().map(StoredTabletFile::getMetaUpdateDelete).sorted().collect(toList());
- jData.txid = FateTxId.formatTid(fateTxId);
- jData.selAll = initiallySelectedAll;
- // ELASITICITY_TODO need the produced json to always be the same when the
input data is the same
- // as its used for comparison. Need unit test to ensure this behavior.
- metadataValue = GSON.get().toJson(jData);
}
- private SelectedFiles(Set<StoredTabletFile> files, boolean
initiallySelectedAll, long fateTxId,
- String metaVal) {
- Preconditions.checkArgument(files != null && !files.isEmpty());
- this.files = files;
- this.initiallySelectedAll = initiallySelectedAll;
- this.fateTxId = fateTxId;
- this.metadataValue = metaVal;
+ private static class SelectedFilesTypeAdapter extends
TypeAdapter<SelectedFiles> {
+
+ @Override
+ public void write(JsonWriter out, SelectedFiles selectedFiles) throws
IOException {
+ out.beginObject();
+ out.name("txid").value(FateTxId.formatTid(selectedFiles.getFateTxId()));
+ out.name("selAll").value(selectedFiles.initiallySelectedAll());
+ out.name("files").beginArray();
+ // sort the data to make serialized json comparable
+
selectedFiles.getFiles().stream().map(StoredTabletFile::getMetaUpdateDelete).sorted()
+ .forEach(file -> {
+ try {
+ out.value(file);
+ } catch (IOException e) {
+ throw new UncheckedIOException(
+ "Failed to add file " + file + " to the JSON files array",
e);
+ }
+ });
+ out.endArray();
+ out.endObject();
+ }
+
+ @Override
+ public SelectedFiles read(JsonReader in) throws IOException {
+ long fateTxId = 0L;
+ boolean selAll = false;
+ List<String> files = new ArrayList<>();
+
+ in.beginObject();
+ while (in.hasNext()) {
+ String name = in.nextName();
+ switch (name) {
+ case "txid":
+ fateTxId = FateTxId.fromString(in.nextString());
+ break;
+ case "selAll":
+ selAll = in.nextBoolean();
+ break;
+ case "files":
+ in.beginArray();
+ while (in.hasNext()) {
+ files.add(in.nextString());
+ }
+ in.endArray();
+ break;
+ default:
+ throw new IllegalArgumentException("Unknown field name : " + name);
+ }
+ }
+ in.endObject();
+
+ Set<StoredTabletFile> tabletFiles =
+
files.stream().map(StoredTabletFile::new).collect(Collectors.toSet());
+
+ return new SelectedFiles(tabletFiles, selAll, fateTxId);
+ }
+
}
public static SelectedFiles from(String json) {
- GSonData jData = GSON.get().fromJson(json, GSonData.class);
- return new SelectedFiles(
-
jData.files.stream().map(StoredTabletFile::new).collect(Collectors.toSet()),
jData.selAll,
- FateTxId.fromString(jData.txid), json);
+ return GSON.fromJson(json, SelectedFiles.class);
}
public Set<StoredTabletFile> getFiles() {
@@ -95,7 +133,29 @@ public class SelectedFiles {
}
public String getMetadataValue() {
- return metadataValue;
+ if (this.metadataValue == null) {
+ // use the custom TypeAdapter to create the json
+ this.metadataValue = GSON.toJson(this);
+ }
+ return this.metadataValue;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ SelectedFiles other = (SelectedFiles) obj;
+ return fateTxId == other.fateTxId && files.equals(other.files)
+ && initiallySelectedAll == other.initiallySelectedAll;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(fateTxId, files, initiallySelectedAll);
}
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/SelectedFilesTest.java
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/SelectedFilesTest.java
new file mode 100644
index 0000000000..555f3b2239
--- /dev/null
+++
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/SelectedFilesTest.java
@@ -0,0 +1,207 @@
+/*
+ * 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.schema;
+
+import static org.apache.accumulo.core.util.LazySingletons.RANDOM;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.accumulo.core.fate.FateTxId;
+import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class SelectedFilesTest {
+
+ final String filePathPrefix =
"hdfs://namenode:9020/accumulo/tables/1/default_tablet/";
+
+ /**
+ * Make sure a SelectedFiles object is equal to another SelectedFiles object
that was created
+ * using its metadata json
+ */
+ @Test
+ public void testSerializationDeserialization() {
+ Set<StoredTabletFile> files = getStoredTabletFiles(2);
+
+ SelectedFiles original = new SelectedFiles(files, true, 12345L);
+
+ String json = original.getMetadataValue();
+ SelectedFiles deserialized = SelectedFiles.from(json);
+
+ assertEquals(original, deserialized);
+ assertEquals(json, deserialized.getMetadataValue());
+ }
+
+ /**
+ * Ensure two SelectedFiles objects created with the same parameters are
equal
+ */
+ @Test
+ public void testEqualSerialization() {
+ Set<StoredTabletFile> files = getStoredTabletFiles(16);
+
+ SelectedFiles sf1 = new SelectedFiles(files, true, 12345L);
+ SelectedFiles sf2 = new SelectedFiles(files, true, 12345L);
+
+ assertEquals(sf1.getMetadataValue(), sf2.getMetadataValue());
+ assertEquals(sf1, sf2);
+ }
+
+ /**
+ * Ensure that SelectedFiles objects created with file arrays of differing
order yet equal entries
+ * are correctly serialized
+ */
+ @Test
+ public void testDifferentFilesOrdering() {
+ Set<StoredTabletFile> files = getStoredTabletFiles(16);
+ SortedSet<StoredTabletFile> sortedFiles = new TreeSet<>(files);
+
+ assertEquals(files, sortedFiles, "Entries in test file sets should be the
same");
+ assertNotEquals(files.toString(), sortedFiles.toString(),
+ "Order of files set should differ for this test case");
+
+ SelectedFiles sf1 = new SelectedFiles(files, false, 654123L);
+ SelectedFiles sf2 = new SelectedFiles(sortedFiles, false, 654123L);
+
+ assertEquals(sf1.getMetadataValue(), sf2.getMetadataValue());
+ assertEquals(sf1, sf2);
+ }
+
+ /**
+ * Ensure that two SelectedFiles objects, one created with a subset of files
present in the other,
+ * are not equal
+ */
+ @Test
+ public void testJsonSuperSetSubset() {
+ Set<StoredTabletFile> filesSuperSet = getStoredTabletFiles(3);
+ Set<StoredTabletFile> filesSubSet = new HashSet<>(filesSuperSet);
+ // Remove an element to create a subset
+ filesSubSet.remove(filesSubSet.iterator().next());
+
+ SelectedFiles superSetSelectedFiles = new SelectedFiles(filesSuperSet,
true, 123456L);
+ SelectedFiles subSetSelectedFiles = new SelectedFiles(filesSubSet, true,
123456L);
+
+ String superSetJson = superSetSelectedFiles.getMetadataValue();
+ String subSetJson = subSetSelectedFiles.getMetadataValue();
+
+ assertNotEquals(superSetJson, subSetJson, "The metadata jsons should have
differing file sets");
+
+ // Deserialize the JSON strings back to SelectedFiles and check for
equality with the original
+ // objects
+ SelectedFiles superSetDeserialized = SelectedFiles.from(superSetJson);
+ SelectedFiles subSetDeserialized = SelectedFiles.from(subSetJson);
+
+ assertEquals(superSetSelectedFiles, superSetDeserialized);
+ assertEquals(subSetSelectedFiles, subSetDeserialized);
+
+ assertNotEquals(superSetDeserialized, subSetDeserialized,
+ "The objects should have differing file sets");
+ }
+
+ private static Stream<Arguments> provideTestJsons() {
+ return Stream.of(Arguments.of("123456", true, 12), Arguments.of("123456",
false, 12),
+ Arguments.of("123456", false, 23), Arguments.of("654321", false, 23),
+ Arguments.of("AE56E", false, 23));
+ }
+
+ /**
+ * Ensure that SelectedFiles objects that are created using {@link
SelectedFiles#from(String)} are
+ * created correctly
+ */
+ @ParameterizedTest
+ @MethodSource("provideTestJsons")
+ public void testJsonStrings(String txid, boolean selAll, int numPaths) {
+ List<String> paths = getFilePaths(numPaths);
+
+ // should be resilient to unordered file arrays
+ Collections.shuffle(paths, RANDOM.get());
+
+ // construct a json from the given parameters
+ String json = getJson(txid, selAll, paths);
+
+ // create a SelectedFiles object using the json
+ SelectedFiles selectedFiles = SelectedFiles.from(json);
+
+ // ensure all parts of the SelectedFiles object are correct
+ assertEquals(Long.parseLong(txid, 16), selectedFiles.getFateTxId());
+ assertEquals(selAll, selectedFiles.initiallySelectedAll());
+ Set<StoredTabletFile> expectedStoredTabletFiles =
filePathsToStoredTabletFiles(paths);
+ assertEquals(expectedStoredTabletFiles, selectedFiles.getFiles());
+
+ Collections.sort(paths);
+ String jsonWithSortedFiles = getJson(txid, selAll, paths);
+ assertEquals(jsonWithSortedFiles, selectedFiles.getMetadataValue());
+ }
+
+ /**
+ * Creates a json suitable to create a SelectedFiles object from. The given
parameters will be
+ * inserted into the returned json String. Example of returned json String:
+ *
+ * <pre>
+ * {
+ * "txid": "FATE[123456]",
+ * "selAll": true,
+ * "files": ["/path/to/file1.rf", "/path/to/file2.rf"]
+ * }
+ * </pre>
+ */
+ private static String getJson(String txid, boolean selAll, List<String>
paths) {
+ String filesJsonArray =
+ paths.stream().map(path -> "'" + path +
"'").collect(Collectors.joining(","));
+ return ("{'txid':'" + FateTxId.formatTid(Long.parseLong(txid, 16)) +
"','selAll':" + selAll
+ + ",'files':[" + filesJsonArray + "]}").replace('\'', '\"');
+ }
+
+ /**
+ * Generate the given number of rfile path strings
+ */
+ private List<String> getFilePaths(int numOfFiles) {
+ List<String> paths = new ArrayList<>(numOfFiles);
+ for (int i = 0; i < numOfFiles; i++) {
+ paths.add(filePathPrefix + "F0000" + i + ".rf");
+ }
+ return paths;
+ }
+
+ /**
+ * Convert a list of file paths to StoredTabletFile objects
+ */
+ private Set<StoredTabletFile> filePathsToStoredTabletFiles(List<String>
filePaths) {
+ return
filePaths.stream().map(StoredTabletFile::new).collect(Collectors.toSet());
+ }
+
+ /**
+ * Generate the given number of StoredTabletFile objects
+ */
+ private Set<StoredTabletFile> getStoredTabletFiles(int numOfFiles) {
+ return filePathsToStoredTabletFiles(getFilePaths(numOfFiles));
+ }
+
+}
