thomasmueller commented on code in PR #1669:
URL: https://github.com/apache/jackrabbit-oak/pull/1669#discussion_r1731725499
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/DocumentStoreIndexerBase.java:
##########
@@ -357,26 +369,37 @@ public void reindex() throws CommitFailedException,
IOException {
indexParallel(flatFileStores, indexer, progressReporter);
} else if (flatFileStores.size() == 1) {
FlatFileStore flatFileStore = flatFileStores.get(0);
- TopKSlowestPaths slowestTopKElements = new
TopKSlowestPaths(TOP_SLOWEST_PATHS_TO_LOG);
- indexer.onIndexingStarting();
- long entryStart = System.nanoTime();
- for (NodeStateEntry entry : flatFileStore) {
- reportDocumentRead(entry.getPath(), progressReporter);
- indexer.index(entry);
- // Avoid calling System.nanoTime() twice per each
entry, by reusing the timestamp taken at the end
- // of indexing an entry as the start time of the
following entry. This is less accurate, because
- // the measured times will also include the
bookkeeping at the end of indexing each entry, but
- // we are only interested in entries that take a
significant time to index, so this extra
- // inaccuracy will not significantly change the
results.
- long entryEnd = System.nanoTime();
- long elapsedMillis = (entryEnd - entryStart) /
1_000_000;
- entryStart = entryEnd;
- slowestTopKElements.add(entry.getPath(),
elapsedMillis);
- if (elapsedMillis > 1000) {
- log.info("Indexing {} took {} ms",
entry.getPath(), elapsedMillis);
+ try (AheadOfTimeBlobDownloader aheadOfTimeBlobDownloader =
getAheadOfTimeBlobDownloaderInterface(flatFileStore, indexer)) {
Review Comment:
The AheadOfTimeBlobDownloader is an interface and the only "real"
implementation requires a FlatFileStore. So another IndexStore is not supported
(which is fine because only the FlatFileStore needs it). But it means all
IndexStores need to support somehow the AheadOfTimeBlobDownloader which is not
true: the TreeStore doesn't need it, so the AheadOfTimeBlobDownloader should
now appear here.
I would instead extend the FlatFileStore such that it can be wrapped by the
AheadOfTimeBlobDownloader and then the AheadOfTimeBlobDownloader could be an
IndexStore.
That way there is no hard dependency on the AheadOfTimeBlobDownloader here.
And so the TreeStore doesn't require a dependency on the
AheadOfTimeBlobDownloader or implement any of it's methods. Instead, we can use
the IndexStore here.
We can discuss in person tomorrow.
--
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]
