This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 3061ff028d Remove redundant LogEntry value (#4071)
3061ff028d is described below
commit 3061ff028df91b7c18a95e24598887590e677f0e
Author: rsingh433 <74160026+rsingh...@users.noreply.github.com>
AuthorDate: Tue Dec 12 13:46:38 2023 -0500
Remove redundant LogEntry value (#4071)
Remove the `LogEntry.getValue()` method and use the column qualifier
for the file path, without the redundant storage in the Value
* Remove `getValue()` method and use empty `new Value()`
* Create addToMutation method to remove boilerplate for storing
the LogEntry in the metadata
* Strictly validate the metadata log entries' column qualifier
This re-applies the reverted change for #3997,
0cdf6dd2de0ca382c3881169bfe90c3bbd4367b3, but without the renaming of
filePath to logReference, since with further analysis of the schema, it
was discovered that these are, in fact, file paths, and not just generic
references to log IDs.
This re-application also removes the metadata constraint that expected
the value to be non-empty, which was overlooked on the first attempt.
---------
Co-authored-by: Christopher Tubbs <ctubb...@apache.org>
---
.../accumulo/core/tabletserver/log/LogEntry.java | 29 ++++++++++------------
.../core/metadata/schema/TabletMetadataTest.java | 11 +++-----
.../core/tabletserver/log/LogEntryTest.java | 5 +---
.../server/constraints/MetadataConstraints.java | 3 ++-
.../org/apache/accumulo/server/fs/VolumeUtil.java | 2 +-
.../server/metadata/TabletMutatorBase.java | 2 +-
.../test/MissingWalHeaderCompletesRecoveryIT.java | 6 ++---
.../apache/accumulo/test/manager/MergeStateIT.java | 3 +--
8 files changed, 25 insertions(+), 36 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
index 9c592b2260..73a9ecf979 100644
--- a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
+++ b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
@@ -23,9 +23,12 @@ import java.util.Objects;
import java.util.UUID;
import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Value;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import org.apache.hadoop.io.Text;
+import com.google.common.base.Preconditions;
import com.google.common.net.HostAndPort;
public class LogEntry {
@@ -47,7 +50,7 @@ public class LogEntry {
* localhost:1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a valid file path.
*
* @param filePath path to validate
- * @throws IllegalArgumentException if the filepath is invalid
+ * @throws IllegalArgumentException if the filePath is invalid
*/
private static void validateFilePath(String filePath) {
String[] parts = filePath.split("/");
@@ -76,12 +79,12 @@ public class LogEntry {
}
/**
- * Make a copy of this LogEntry but replace the file path.
+ * Add LogEntry information to the provided mutation.
*
- * @param filePath path to use
+ * @param mutation the mutation to update
*/
- public LogEntry switchFile(String filePath) {
- return new LogEntry(filePath);
+ public void addToMutation(Mutation mutation) {
+
mutation.at().family(LogColumnFamily.NAME).qualifier(getColumnQualifier()).put(new
Value());
}
@Override
@@ -107,13 +110,11 @@ public class LogEntry {
}
public static LogEntry fromMetaWalEntry(Entry<Key,Value> entry) {
- final Value value = entry.getValue();
-
- String filePath = value.toString();
-
- validateFilePath(filePath);
-
- return new LogEntry(filePath);
+ String qualifier = entry.getKey().getColumnQualifier().toString();
+ String[] parts = qualifier.split("/", 2);
+ Preconditions.checkArgument(parts.length == 2 && parts[0].equals("-"),
+ "Malformed write-ahead log %s", qualifier);
+ return new LogEntry(parts[1]);
}
public String getUniqueID() {
@@ -125,8 +126,4 @@ public class LogEntry {
return new Text("-/" + filePath);
}
- public Value getValue() {
- return new Value(filePath);
- }
-
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
index d547b8596d..a4cb1b583c 100644
---
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
@@ -59,7 +59,6 @@ import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Cu
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily;
-import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily;
@@ -104,11 +103,9 @@ public class TabletMetadataTest {
mutation.at().family(LastLocationColumnFamily.NAME).qualifier("s000").put("server2:8555");
LogEntry le1 = new LogEntry("localhost:8020/" + UUID.randomUUID());
-
mutation.at().family(LogColumnFamily.NAME).qualifier(le1.getColumnQualifier())
- .put(le1.getValue());
+ le1.addToMutation(mutation);
LogEntry le2 = new LogEntry("localhost:8020/" + UUID.randomUUID());
-
mutation.at().family(LogColumnFamily.NAME).qualifier(le2.getColumnQualifier())
- .put(le2.getValue());
+ le2.addToMutation(mutation);
StoredTabletFile sf1 = StoredTabletFile.of(new
Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf"));
StoredTabletFile sf2 = StoredTabletFile.of(new
Path("hdfs://nn1/acc/tables/1/t-0001/sf2.rf"));
@@ -140,8 +137,8 @@ public class TabletMetadataTest {
assertEquals(HostAndPort.fromParts("server2", 8555),
tm.getLast().getHostAndPort());
assertEquals("s000", tm.getLast().getSession());
assertEquals(LocationType.LAST, tm.getLast().getType());
- assertEquals(Set.of(le1.getValue(), le2.getValue()),
- tm.getLogs().stream().map(LogEntry::getValue).collect(toSet()));
+ assertEquals(Set.of(le1.getColumnQualifier(), le2.getColumnQualifier()),
+
tm.getLogs().stream().map(LogEntry::getColumnQualifier).collect(toSet()));
assertEquals(extent.prevEndRow(), tm.getPrevEndRow());
assertEquals(extent.tableId(), tm.getTableId());
assertTrue(tm.sawPrevEndRow());
diff --git
a/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
b/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
index 302021121e..3adec0aa0c 100644
---
a/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
@@ -55,17 +55,15 @@ public class LogEntryTest {
assertEquals(validFilename, one.getFilePath());
assertEquals(new Text("-/" + validFilename), one.getColumnQualifier());
assertEquals(validUUID.toString(), one.getUniqueID());
- assertEquals(new Value(validFilename), one.getValue());
// test from metadata entry
LogEntry two = LogEntry.fromMetaWalEntry(new
AbstractMap.SimpleImmutableEntry<>(
- new Key(new Text("1<"), new Text("log"), one.getColumnQualifier()),
one.getValue()));
+ new Key(new Text("1<"), new Text("log"), one.getColumnQualifier()),
new Value("unused")));
assertNotSame(one, two);
assertEquals(one.toString(), two.toString());
assertEquals(one.getFilePath(), two.getFilePath());
assertEquals(one.getColumnQualifier(), two.getColumnQualifier());
assertEquals(one.getUniqueID(), two.getUniqueID());
- assertEquals(one.getValue(), two.getValue());
assertEquals(one, two);
}
@@ -79,7 +77,6 @@ public class LogEntryTest {
assertEquals(one.getFilePath(), two.getFilePath());
assertEquals(one.getColumnQualifier(), two.getColumnQualifier());
assertEquals(one.getUniqueID(), two.getUniqueID());
- assertEquals(one.getValue(), two.getValue());
assertEquals(one, two);
assertEquals(one, one);
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java
b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java
index 617bf20e86..3579fdbc6e 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/constraints/MetadataConstraints.java
@@ -213,7 +213,8 @@ public class MetadataConstraints implements Constraint {
continue;
}
- if (columnUpdate.getValue().length == 0 &&
!columnFamily.equals(ScanFileColumnFamily.NAME)) {
+ if (columnUpdate.getValue().length == 0 &&
!(columnFamily.equals(ScanFileColumnFamily.NAME)
+ || columnFamily.equals(LogColumnFamily.NAME))) {
violations = addViolation(violations, 6);
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
index 98920aa2ea..4021840869 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
@@ -103,7 +103,7 @@ public class VolumeUtil {
return null;
}
- LogEntry newLogEntry = le.switchFile(switchedString);
+ LogEntry newLogEntry = new LogEntry(switchedString);
log.trace("Switched {} to {}", le, newLogEntry);
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
index 73776db9b9..4f7f9c230c 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
@@ -175,7 +175,7 @@ public abstract class TabletMutatorBase implements
Ample.TabletMutator {
@Override
public Ample.TabletMutator putWal(LogEntry logEntry) {
Preconditions.checkState(updatesEnabled, "Cannot make updates after
calling mutate.");
- mutation.put(LogColumnFamily.NAME, logEntry.getColumnQualifier(),
logEntry.getValue());
+ logEntry.addToMutation(mutation);
return this;
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
b/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
index 983f9217d8..f6711ff223 100644
---
a/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
@@ -146,8 +146,7 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
- logEntry.getValue());
+ logEntry.addToMutation(m);
try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
bw.addMutation(m);
@@ -205,8 +204,7 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
- logEntry.getValue());
+ logEntry.addToMutation(m);
try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
bw.addMutation(m);
diff --git
a/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
b/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
index 699a8f7eaf..40c2e07b5f 100644
--- a/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
@@ -204,8 +204,7 @@ public class MergeStateIT extends ConfigurableMacBase {
KeyExtent ke = new KeyExtent(tableId, new Text("t"), new Text("p"));
m = new Mutation(ke.toMetaRow());
LogEntry logEntry = new LogEntry("localhost:1234/" + UUID.randomUUID());
-
m.at().family(LogColumnFamily.NAME).qualifier(logEntry.getColumnQualifier())
- .put(logEntry.getValue());
+ logEntry.addToMutation(m);
update(accumuloClient, m);
// Verify state is still WAITING_FOR_OFFLINE
