keith-turner commented on code in PR #3509:
URL: https://github.com/apache/accumulo/pull/3509#discussion_r1237100573
##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletFile.java:
##########
@@ -49,33 +51,54 @@ public class TabletFile implements Comparable<TabletFile> {
private static final Logger log = LoggerFactory.getLogger(TabletFile.class);
+ private static final String HDFS_TABLES_DIR_NAME =
HDFS_TABLES_DIR.substring(1);
+
/**
* Construct new tablet file using a Path. Used in the case where we had to
use Path object to
* qualify an absolute path or create a new file.
*/
public TabletFile(Path metaPath) {
this.metaPath = Objects.requireNonNull(metaPath);
- String errorMsg = "Missing or invalid part of tablet file metadata entry:
" + metaPath;
+ final String errorMsg = "Missing or invalid part of tablet file metadata
entry: " + metaPath;
log.trace("Parsing TabletFile from {}", metaPath);
- // use Path object to step backwards from the filename through all the
parts
- this.fileName = metaPath.getName();
- ValidationUtil.validateFileName(fileName);
+ // File name construct: <volume>/<tablePath>/<tableId>/<tablet>/<file>
+ // Example: hdfs://namenode:9020/accumulo/tables/1/default_tablet/F00001.rf
+ final URI uri = this.metaPath.toUri();
+ final String path = uri.getPath();
+ final String[] parts = path.split("/");
+ final int numParts = parts.length;
- Path tabletDirPath = Objects.requireNonNull(metaPath.getParent(),
errorMsg);
+ if (numParts < 4) {
+ throw new IllegalArgumentException(errorMsg);
+ }
- Path tableIdPath = Objects.requireNonNull(tabletDirPath.getParent(),
errorMsg);
- var id = tableIdPath.getName();
+ this.fileName = parts[numParts - 1];
+ final String tabletDirectory = parts[numParts - 2];
+ final String tableId = parts[numParts - 3];
+ final String tablesPath = parts[numParts - 4];
+ if (StringUtils.isBlank(fileName) || StringUtils.isBlank(tabletDirectory)
+ || StringUtils.isBlank(tableId) || StringUtils.isBlank(tablesPath)) {
+ throw new IllegalArgumentException(errorMsg);
+ }
+ ValidationUtil.validateFileName(fileName);
+ Preconditions.checkArgument(tablesPath.equals(HDFS_TABLES_DIR_NAME),
+ "tables directory name is not " + HDFS_TABLES_DIR_NAME + ", is " +
tablesPath);
- Path tablePath = Objects.requireNonNull(tableIdPath.getParent(), errorMsg);
- String tpString = "/" + tablePath.getName();
- Preconditions.checkArgument(tpString.equals(HDFS_TABLES_DIR), errorMsg);
+ // determine where file path starts, the rest is the volume
+ final String filePath =
+ HDFS_TABLES_DIR + "/" + tableId + "/" + tabletDirectory + "/" +
this.fileName;
+ final String uriString = uri.toString();
+ int idx = uriString.indexOf(filePath);
Review Comment:
Could possible use the String.lastIndexOf method here. Not sure if it would
be faster though. It would cover the case where filePath occurs twice in the
original uri, we want to find the last occurrence.
A long long time ago I experimented with making the key compare in reverese
order because for sorted data inequality is more likely at the end of a string.
However comparing in reverse order was slower than comparing in forward order.
I suspect most optimizations are geared twoards iterating forward and that is
why it was slower.
So not sure about this suggestion for speed, but it does seem better for
correctness.
--
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]
