dlmarion commented on code in PR #3385:
URL: https://github.com/apache/accumulo/pull/3385#discussion_r1188850393
##########
core/src/main/java/org/apache/accumulo/core/gc/ReferenceDirectory.java:
##########
@@ -46,8 +47,8 @@ public String getTabletDir() {
* A Tablet directory should have a metadata entry equal to the dirName.
*/
@Override
- public String getMetadataEntry() {
- if (!tabletDir.equals(metadataEntry)) {
+ public TabletFileMetadataEntry getMetadataEntry() {
+ if (!tabletDir.equals(metadataEntry.getFilePathString())) {
Review Comment:
Since the variables are `final`, I think this check should be done in the
constructor.
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/ExternalCompactionMetadata.java:
##########
@@ -129,10 +129,11 @@ private static class GSonData {
public String toJson() {
GSonData jData = new GSonData();
- jData.inputs =
jobFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.nextFiles =
-
nextFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.tmp = compactTmpName.getMetaInsert();
+ jData.inputs = jobFiles.stream()
+ .map(tabletFile ->
tabletFile.getMetaUpdateDelete().getMetaString()).collect(toList());
Review Comment:
This breaks when/if `getMetaString` changes to include the file range.
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/ExternalCompactionMetadata.java:
##########
@@ -129,10 +129,11 @@ private static class GSonData {
public String toJson() {
GSonData jData = new GSonData();
- jData.inputs =
jobFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.nextFiles =
-
nextFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.tmp = compactTmpName.getMetaInsert();
+ jData.inputs = jobFiles.stream()
+ .map(tabletFile ->
tabletFile.getMetaUpdateDelete().getMetaString()).collect(toList());
+ jData.nextFiles = nextFiles.stream()
+ .map(tabletFile ->
tabletFile.getMetaUpdateDelete().getMetaString()).collect(toList());
Review Comment:
This breaks when/if `getMetaString` changes to include the file range.
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -33,39 +35,47 @@
* in Upgrader9to10.upgradeRelativePaths()
*/
public class StoredTabletFile extends TabletFile {
- private final String metadataEntry;
+ private final TabletFileMetadataEntry metadataEntry;
Review Comment:
Is this variable a duplicate of `TabletFile.normalizedPath`?
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -33,39 +35,47 @@
* in Upgrader9to10.upgradeRelativePaths()
*/
public class StoredTabletFile extends TabletFile {
- private final String metadataEntry;
+ private final TabletFileMetadataEntry metadataEntry;
+
+ /**
+ * Construct a tablet file using the {@link TabletFileMetadataEntry} from
the metadata. Preserve
+ * the exact string so the entry can be deleted.
+ */
+ public StoredTabletFile(final TabletFileMetadataEntry metadataEntry) {
+ super(Objects.requireNonNull(metadataEntry).getFilePath());
+ this.metadataEntry = metadataEntry;
+ }
/**
* Construct a tablet file using the string read from the metadata. Preserve
the exact string so
* the entry can be deleted.
*/
- public StoredTabletFile(String metadataEntry) {
- super(new Path(metadataEntry));
- this.metadataEntry = metadataEntry;
+ public StoredTabletFile(final String metadataEntry) {
+ this(TabletFileMetadataEntry.of(metadataEntry));
}
/**
- * Return the exact string that is stored in the metadata table. This is
important for updating
- * and deleting metadata entries. If the exact string is not used, erroneous
entries can pollute
- * the metadata table.
+ * Return the {@link TabletFileMetadataEntry} which contains the exact
string that is stored in
+ * the metadata table. This is important for updating and deleting metadata
entries. If the exact
+ * string is not used, erroneous entries can pollute the metadata table.
*/
- public String getMetaUpdateDelete() {
+ public TabletFileMetadataEntry getMetaUpdateDelete() {
return metadataEntry;
}
/**
* Return a new Text object of {@link #getMetaUpdateDelete()}
*/
public Text getMetaUpdateDeleteText() {
- return new Text(getMetaUpdateDelete());
+ return getMetaUpdateDelete().getMetaText();
Review Comment:
there is no difference between this method and `TabletFile.getMetaInsert`.
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -345,16 +345,16 @@ public CompactionStats call() throws IOException,
CompactionCanceledException {
readers.add(reader);
InterruptibleIterator iter = new ProblemReportingIterator(context,
extent.tableId(),
- mapFile.getPathStr(), false, reader);
+ mapFile.getPathStr().toString(), false, reader);
iter = filesToCompact.get(mapFile).wrapFileIterator(iter);
iters.add(iter);
} catch (Exception e) {
- ProblemReports.getInstance(context).report(
- new ProblemReport(extent.tableId(), ProblemType.FILE_READ,
mapFile.getPathStr(), e));
+ ProblemReports.getInstance(context).report(new
ProblemReport(extent.tableId(),
+ ProblemType.FILE_READ, mapFile.getPathStr().toString(), e));
Review Comment:
`getPathStr` is already a String.
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/ExternalCompactionMetadata.java:
##########
@@ -129,10 +129,11 @@ private static class GSonData {
public String toJson() {
GSonData jData = new GSonData();
- jData.inputs =
jobFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.nextFiles =
-
nextFiles.stream().map(StoredTabletFile::getMetaUpdateDelete).collect(toList());
- jData.tmp = compactTmpName.getMetaInsert();
+ jData.inputs = jobFiles.stream()
+ .map(tabletFile ->
tabletFile.getMetaUpdateDelete().getMetaString()).collect(toList());
+ jData.nextFiles = nextFiles.stream()
+ .map(tabletFile ->
tabletFile.getMetaUpdateDelete().getMetaString()).collect(toList());
+ jData.tmp = compactTmpName.getMetaInsert().getMetaString();
Review Comment:
This breaks when/if `getMetaString` changes to include the file range.
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -345,16 +345,16 @@ public CompactionStats call() throws IOException,
CompactionCanceledException {
readers.add(reader);
InterruptibleIterator iter = new ProblemReportingIterator(context,
extent.tableId(),
- mapFile.getPathStr(), false, reader);
+ mapFile.getPathStr().toString(), false, reader);
Review Comment:
`getPathStr` is already a String.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
