Skip to content

Commit bf61fac

Browse files
authored
Fixed bug in TabletsMetadata.fetch (apache#5342)
A bug was introduced in apache#5335 to TabletsMetadata.fetch where the columns being passed as arguments to the function where not being used. This has the side effect of fetching all columns in the TabletMetadata which was causing some ITs to time out. The ITs in these cases were fetching and counting specific columns in the TabletMetadata. This bug also highlighted another bug in CloneIT where an invalid directory was being inserted into the TabletMetadata which would then subsequently fail when fetched. The directory column name was being validated on most, but not all, places where it was being inserted into the TabletMetadata. I added validation to the ServerColumnFamily.DIRECTORY_COLUMN.put method and fixed CloneIT.
1 parent 80b8fbc commit bf61fac

File tree

6 files changed

+34
-30
lines changed

6 files changed

+34
-30
lines changed

core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,13 @@ public static class ServerColumnFamily {
203203
* Holds the location of the tablet in the DFS file system
204204
*/
205205
public static final String DIRECTORY_QUAL = "dir";
206-
public static final ColumnFQ DIRECTORY_COLUMN = new ColumnFQ(NAME, new Text(DIRECTORY_QUAL));
206+
public static final ColumnFQ DIRECTORY_COLUMN = new ColumnFQ(NAME, new Text(DIRECTORY_QUAL)) {
207+
@Override
208+
public void put(Mutation m, Value v) {
209+
validateDirCol(v.toString());
210+
super.put(m, v);
211+
}
212+
};
207213
/**
208214
* Initial tablet directory name for the default tablet in all tables
209215
*/

core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java

-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@
7373
import org.apache.accumulo.core.metadata.StoredTabletFile;
7474
import org.apache.accumulo.core.metadata.SuspendingTServer;
7575
import org.apache.accumulo.core.metadata.TServerInstance;
76-
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
7776
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily;
7877
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily;
7978
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CompactedColumnFamily;

core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMutatorBase.java

-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ public T putPrevEndRow(Text per) {
9090

9191
@Override
9292
public T putDirName(String dirName) {
93-
ServerColumnFamily.validateDirCol(dirName);
9493
Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate.");
9594
ServerColumnFamily.DIRECTORY_COLUMN.put(mutation, new Value(dirName));
9695
return getThis();

core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static com.google.common.base.Preconditions.checkState;
2222
import static java.util.stream.Collectors.groupingBy;
2323
import static java.util.stream.Collectors.toList;
24-
import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW;
2524

2625
import java.io.IOException;
2726
import java.io.UncheckedIOException;
@@ -305,7 +304,7 @@ public Options checkConsistency() {
305304
@Override
306305
public Options fetch(ColumnType... colsToFetch) {
307306
Preconditions.checkArgument(colsToFetch.length > 0);
308-
for (var col : fetchedCols) {
307+
for (var col : colsToFetch) {
309308
fetchedCols.add(col);
310309
var qualifier = ColumnType.resolveQualifier(col);
311310
if (qualifier != null) {

server/base/src/test/java/org/apache/accumulo/server/constraints/MetadataConstraintsTest.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,8 @@ public void testDirectoryColumn() {
634634
assertTrue(violations.isEmpty());
635635

636636
m = new Mutation(new Text("0;foo"));
637-
ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value("/invalid"));
637+
m.put(ServerColumnFamily.DIRECTORY_COLUMN.getColumnFamily(),
638+
ServerColumnFamily.DIRECTORY_COLUMN.getColumnQualifier(), new Value("/invalid"));
638639
violations = mc.check(createEnv(), m);
639640
assertFalse(violations.isEmpty());
640641
assertEquals(1, violations.size());

test/src/main/java/org/apache/accumulo/test/CloneIT.java

+24-24
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void testNoFiles() throws Exception {
6666
Mutation mut = TabletColumnFamily.createPrevRowMutation(ke);
6767

6868
ServerColumnFamily.TIME_COLUMN.put(mut, new Value("M0"));
69-
ServerColumnFamily.DIRECTORY_COLUMN.put(mut, new Value("/default_tablet"));
69+
ServerColumnFamily.DIRECTORY_COLUMN.put(mut, new Value("default_tablet"));
7070

7171
try (BatchWriter bw1 = client.createBatchWriter(tableName)) {
7272
bw1.addMutation(mut);
@@ -95,7 +95,7 @@ public void testFilesChange(Range range1, Range range2) throws Exception {
9595
Mutation mut = TabletColumnFamily.createPrevRowMutation(ke);
9696

9797
ServerColumnFamily.TIME_COLUMN.put(mut, new Value("M0"));
98-
ServerColumnFamily.DIRECTORY_COLUMN.put(mut, new Value("/default_tablet"));
98+
ServerColumnFamily.DIRECTORY_COLUMN.put(mut, new Value("default_tablet"));
9999
mut.put(DataFileColumnFamily.NAME.toString(),
100100
getMetadata(filePrefix + "/default_tablet/0_0.rf", range1),
101101
new DataFileValue(1, 200).encodeAsString());
@@ -155,17 +155,17 @@ public void testSplit1(Range range) throws Exception {
155155

156156
try (BatchWriter bw1 = client.createBatchWriter(tableName);
157157
BatchWriter bw2 = client.createBatchWriter(tableName)) {
158-
bw1.addMutation(createTablet("0", null, null, "/default_tablet",
158+
bw1.addMutation(createTablet("0", null, null, "default_tablet",
159159
filePrefix + "/default_tablet/0_0.rf", range));
160160

161161
bw1.flush();
162162

163163
MetadataTableUtil.initializeClone(tableName, TableId.of("0"), TableId.of("1"), client, bw2);
164164

165-
bw1.addMutation(createTablet("0", "m", null, "/default_tablet",
165+
bw1.addMutation(createTablet("0", "m", null, "default_tablet",
166166
filePrefix + "/default_tablet/0_0.rf", range));
167167
bw1.addMutation(
168-
createTablet("0", null, "m", "/t-1", filePrefix + "/default_tablet/0_0.rf", range));
168+
createTablet("0", null, "m", "t-1", filePrefix + "/default_tablet/0_0.rf", range));
169169

170170
bw1.flush();
171171

@@ -203,17 +203,17 @@ public void testSplit2(Range range) throws Exception {
203203

204204
try (BatchWriter bw1 = client.createBatchWriter(tableName);
205205
BatchWriter bw2 = client.createBatchWriter(tableName)) {
206-
bw1.addMutation(createTablet("0", null, null, "/default_tablet",
206+
bw1.addMutation(createTablet("0", null, null, "default_tablet",
207207
filePrefix + "/default_tablet/0_0.rf", range));
208208

209209
bw1.flush();
210210

211211
MetadataTableUtil.initializeClone(tableName, TableId.of("0"), TableId.of("1"), client, bw2);
212212

213-
bw1.addMutation(createTablet("0", "m", null, "/default_tablet",
213+
bw1.addMutation(createTablet("0", "m", null, "default_tablet",
214214
filePrefix + "/default_tablet/1_0.rf", range));
215215
Mutation mut3 =
216-
createTablet("0", null, "m", "/t-1", filePrefix + "/default_tablet/1_0.rf", range);
216+
createTablet("0", null, "m", "t-1", filePrefix + "/default_tablet/1_0.rf", range);
217217
mut3.putDelete(DataFileColumnFamily.NAME.toString(),
218218
getMetadata(filePrefix + "/default_tablet/0_0.rf", range));
219219
bw1.addMutation(mut3);
@@ -285,17 +285,17 @@ public void testSplit3(Range range1, Range range2, Range range3) throws Exceptio
285285

286286
try (BatchWriter bw1 = client.createBatchWriter(tableName);
287287
BatchWriter bw2 = client.createBatchWriter(tableName)) {
288-
bw1.addMutation(createTablet("0", "m", null, "/d1", filePrefix + "/d1/file1.rf", range1));
289-
bw1.addMutation(createTablet("0", null, "m", "/d2", filePrefix + "/d2/file2.rf", range2));
288+
bw1.addMutation(createTablet("0", "m", null, "d1", filePrefix + "/d1/file1.rf", range1));
289+
bw1.addMutation(createTablet("0", null, "m", "d2", filePrefix + "/d2/file2.rf", range2));
290290

291291
bw1.flush();
292292

293293
MetadataTableUtil.initializeClone(tableName, TableId.of("0"), TableId.of("1"), client, bw2);
294294

295-
bw1.addMutation(createTablet("0", "f", null, "/d1", filePrefix + "/d1/file3.rf", range3));
296-
bw1.addMutation(createTablet("0", "m", "f", "/d3", filePrefix + "/d1/file1.rf", range1));
297-
bw1.addMutation(createTablet("0", "s", "m", "/d2", filePrefix + "/d2/file2.rf", range2));
298-
bw1.addMutation(createTablet("0", null, "s", "/d4", filePrefix + "/d2/file2.rf", range2));
295+
bw1.addMutation(createTablet("0", "f", null, "d1", filePrefix + "/d1/file3.rf", range3));
296+
bw1.addMutation(createTablet("0", "m", "f", "d3", filePrefix + "/d1/file1.rf", range1));
297+
bw1.addMutation(createTablet("0", "s", "m", "d2", filePrefix + "/d2/file2.rf", range2));
298+
bw1.addMutation(createTablet("0", null, "s", "d4", filePrefix + "/d2/file2.rf", range2));
299299

300300
bw1.flush();
301301

@@ -335,8 +335,8 @@ public void testClonedMarker(Range range1, Range range2, Range range3) throws Ex
335335

336336
try (BatchWriter bw1 = client.createBatchWriter(tableName);
337337
BatchWriter bw2 = client.createBatchWriter(tableName)) {
338-
bw1.addMutation(createTablet("0", "m", null, "/d1", filePrefix + "/d1/file1.rf", range1));
339-
bw1.addMutation(createTablet("0", null, "m", "/d2", filePrefix + "/d2/file2.rf", range2));
338+
bw1.addMutation(createTablet("0", "m", null, "d1", filePrefix + "/d1/file1.rf", range1));
339+
bw1.addMutation(createTablet("0", null, "m", "d2", filePrefix + "/d2/file2.rf", range2));
340340

341341
bw1.flush();
342342

@@ -347,10 +347,10 @@ public void testClonedMarker(Range range1, Range range2, Range range3) throws Ex
347347

348348
bw1.flush();
349349

350-
bw1.addMutation(createTablet("0", "f", null, "/d1", filePrefix + "/d1/file3.rf", range3));
351-
bw1.addMutation(createTablet("0", "m", "f", "/d3", filePrefix + "/d1/file1.rf", range1));
352-
bw1.addMutation(createTablet("0", "s", "m", "/d2", filePrefix + "/d2/file3.rf", range3));
353-
bw1.addMutation(createTablet("0", null, "s", "/d4", filePrefix + "/d4/file3.rf", range3));
350+
bw1.addMutation(createTablet("0", "f", null, "d1", filePrefix + "/d1/file3.rf", range3));
351+
bw1.addMutation(createTablet("0", "m", "f", "d3", filePrefix + "/d1/file1.rf", range1));
352+
bw1.addMutation(createTablet("0", "s", "m", "d2", filePrefix + "/d2/file3.rf", range3));
353+
bw1.addMutation(createTablet("0", null, "s", "d4", filePrefix + "/d4/file3.rf", range3));
354354

355355
bw1.flush();
356356

@@ -363,7 +363,7 @@ public void testClonedMarker(Range range1, Range range2, Range range3) throws Ex
363363

364364
bw1.flush();
365365

366-
bw1.addMutation(createTablet("0", "m", "f", "/d3", filePrefix + "/d1/file3.rf", range3));
366+
bw1.addMutation(createTablet("0", "m", "f", "d3", filePrefix + "/d1/file3.rf", range3));
367367

368368
bw1.flush();
369369

@@ -405,15 +405,15 @@ public void testMerge(Range range1, Range range2) throws Exception {
405405

406406
try (BatchWriter bw1 = client.createBatchWriter(tableName);
407407
BatchWriter bw2 = client.createBatchWriter(tableName)) {
408-
bw1.addMutation(createTablet("0", "m", null, "/d1", filePrefix + "/d1/file1.rf", range1));
409-
bw1.addMutation(createTablet("0", null, "m", "/d2", filePrefix + "/d2/file2.rf", range2));
408+
bw1.addMutation(createTablet("0", "m", null, "d1", filePrefix + "/d1/file1.rf", range1));
409+
bw1.addMutation(createTablet("0", null, "m", "d2", filePrefix + "/d2/file2.rf", range2));
410410

411411
bw1.flush();
412412

413413
MetadataTableUtil.initializeClone(tableName, TableId.of("0"), TableId.of("1"), client, bw2);
414414

415415
bw1.addMutation(deleteTablet("0", "m", null, filePrefix + "/d1/file1.rf", range1));
416-
Mutation mut = createTablet("0", null, null, "/d2", filePrefix + "/d2/file2.rf", range2);
416+
Mutation mut = createTablet("0", null, null, "d2", filePrefix + "/d2/file2.rf", range2);
417417
mut.put(DataFileColumnFamily.NAME.toString(),
418418
getMetadata(filePrefix + "/d1/file1.rf", range1),
419419
new DataFileValue(10, 200).encodeAsString());

0 commit comments

Comments
 (0)