xiangfu0 commented on code in PR #20010:
URL: https://github.com/apache/pinot/pull/20010#discussion_r4358835330
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java:
##########
@@ -664,6 +665,81 @@ private String extractSegmentName(@Nullable String
filePath) {
return segmentName;
}
+ /// Lists the table data dir (`<dataDir>/<rawTableName>/`) without
recursion. Returns null when the directory does
+ /// not exist: the data dir is created when the first segment is pushed, so
it is legitimately absent for a table
+ /// that has never had a segment in deep store, and listing a non-existent
directory fails on most file systems.
+ @Nullable
+ protected List<FileMetadata> listDeepStoreTableDir(URI tableDataUri)
+ throws IOException {
+ PinotFS pinotFS = PinotFSFactory.create(tableDataUri.getScheme());
+ if (!pinotFS.exists(tableDataUri)) {
+ return null;
+ }
+ long startTimeMs = System.currentTimeMillis();
+ List<FileMetadata> deepstoreFiles =
pinotFS.listFilesWithMetadata(tableDataUri, false);
+ LOGGER.info("Found: {} segments in deepstore dir: {}. Time taken to list
segments: {} ms", deepstoreFiles.size(),
+ tableDataUri, System.currentTimeMillis() - startTimeMs);
+ return deepstoreFiles;
+ }
+
+ /// Per-table state of one run. Memoizes the segment ZK metadata list and
the deep-store directory listing, the two
+ /// reads that dominate the cost on a large table, so each happens at most
once per table per run and every consumer
+ /// in the same `processTable` call sees the same snapshot. A context lives
for one `processTable` call only, so the
+ /// memory held for one table is released before the next table is
processed. Not thread-safe: the task processes
+ /// tables sequentially under its run lock.
+ public final class TableRunContext {
+ private final String _tableNameWithType;
+ private final String _rawTableName;
+ @Nullable
+ private URI _tableDataUri;
+ @Nullable
+ private List<SegmentZKMetadata> _segmentsZKMetadata;
+ private boolean _deepStoreListed;
+ @Nullable
+ private List<FileMetadata> _deepStoreFiles;
+
+ TableRunContext(String tableNameWithType) {
+ _tableNameWithType = tableNameWithType;
+ _rawTableName = TableNameBuilder.extractRawTableName(tableNameWithType);
+ }
+
+ public String getTableNameWithType() {
+ return _tableNameWithType;
+ }
+
+ public String getRawTableName() {
+ return _rawTableName;
+ }
+
+ /// `<dataDir>/<rawTableName>`, the directory that holds the deep-store
copies of the table's segments.
+ public URI getTableDataUri() {
+ if (_tableDataUri == null) {
+ _tableDataUri =
URIUtils.getUri(_pinotHelixResourceManager.getDataDir(), _rawTableName);
+ }
+ return _tableDataUri;
+ }
+
+ /// Segment ZK metadata of the typed table, read on the first call and
reused afterwards.
+ public List<SegmentZKMetadata> getSegmentsZKMetadata() {
+ if (_segmentsZKMetadata == null) {
Review Comment:
Minor: this `== null` guard is correct only because `getSegmentsZKMetadata`
never returns null (it returns `List.of()` for a missing path), whereas the
deep-store listing needs the separate `_deepStoreListed` boolean to cache a
null result. Worth a one-line comment here noting that the ZK read is never
null, so the asymmetry with `getDeepStoreFiles()` is intentional rather than an
oversight.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
