thomasmueller commented on code in PR #826:
URL: https://github.com/apache/jackrabbit-oak/pull/826#discussion_r1082172951
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java:
##########
@@ -222,26 +235,53 @@ public List<FlatFileStore> buildList(IndexHelper
indexHelper, IndexerSupport ind
return storeList;
}
- private File createdSortedStoreFile() throws IOException,
CompositeException {
+ /**
+ * Returns the existing list of store files if it can read from system
property OAK_INDEXER_SORTED_FILE_PATH which
+ * defines the existing folder where the flat file store files are
present. Will throw an exception if it cannot
+ * read or the path in the system property is not a directory.
+ * If the system property OAK_INDEXER_SORTED_FILE_PATH in undefined, or it
cannot read relevant files it
+ * initializes the flat file store.
+ *
+ * @return list of flat files
+ * @throws IOException
+ * @throws CompositeException
+ */
+ private List<File> createdSortedStoreFiles() throws IOException,
CompositeException {
+ // Check system property defined path
String sortedFilePath =
System.getProperty(OAK_INDEXER_SORTED_FILE_PATH);
- if (sortedFilePath != null) {
- File sortedFile = new File(sortedFilePath);
- if (sortedFile.exists() && sortedFile.isFile() &&
sortedFile.canRead()) {
- log.info("Reading from provided sorted file [{}] (via system
property '{}')",
- sortedFile.getAbsolutePath(),
OAK_INDEXER_SORTED_FILE_PATH);
- return sortedFile;
- } else {
- String msg = String.format("Cannot read sorted file at [%s]
configured via system property '%s'",
- sortedFile.getAbsolutePath(),
OAK_INDEXER_SORTED_FILE_PATH);
- throw new IllegalArgumentException(msg);
+ if (StringUtils.isNotBlank(sortedFilePath)) {
+ List<File> files = getFiles(sortedFilePath);
+ if (files != null) {
+ return files;
+ }
+ }
+
+ // Initialize the flat file store again
+
+ createStoreDir();
+ org.apache.jackrabbit.oak.index.indexer.document.flatfile.SortStrategy
strategy = createSortStrategy(flatFileStoreDir);
Review Comment:
Couldn't we import the class?
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java:
##########
@@ -222,26 +235,53 @@ public List<FlatFileStore> buildList(IndexHelper
indexHelper, IndexerSupport ind
return storeList;
}
- private File createdSortedStoreFile() throws IOException,
CompositeException {
+ /**
+ * Returns the existing list of store files if it can read from system
property OAK_INDEXER_SORTED_FILE_PATH which
+ * defines the existing folder where the flat file store files are
present. Will throw an exception if it cannot
+ * read or the path in the system property is not a directory.
+ * If the system property OAK_INDEXER_SORTED_FILE_PATH in undefined, or it
cannot read relevant files it
+ * initializes the flat file store.
+ *
+ * @return list of flat files
+ * @throws IOException
+ * @throws CompositeException
+ */
+ private List<File> createdSortedStoreFiles() throws IOException,
CompositeException {
+ // Check system property defined path
String sortedFilePath =
System.getProperty(OAK_INDEXER_SORTED_FILE_PATH);
- if (sortedFilePath != null) {
- File sortedFile = new File(sortedFilePath);
- if (sortedFile.exists() && sortedFile.isFile() &&
sortedFile.canRead()) {
- log.info("Reading from provided sorted file [{}] (via system
property '{}')",
- sortedFile.getAbsolutePath(),
OAK_INDEXER_SORTED_FILE_PATH);
- return sortedFile;
- } else {
- String msg = String.format("Cannot read sorted file at [%s]
configured via system property '%s'",
- sortedFile.getAbsolutePath(),
OAK_INDEXER_SORTED_FILE_PATH);
- throw new IllegalArgumentException(msg);
+ if (StringUtils.isNotBlank(sortedFilePath)) {
+ List<File> files = getFiles(sortedFilePath);
+ if (files != null) {
+ return files;
+ }
+ }
+
+ // Initialize the flat file store again
+
+ createStoreDir();
+ org.apache.jackrabbit.oak.index.indexer.document.flatfile.SortStrategy
strategy = createSortStrategy(flatFileStoreDir);
+ File result = strategy.createSortedStoreFile();
+ entryCount = strategy.getEntryCount();
+ return Collections.singletonList(result);
+ }
+
+ @Nullable
+ private List<File> getFiles(String sortedFilePath) {
+ File sortedDir = new File(sortedFilePath);
+ if (sortedDir.exists() && sortedDir.canRead() &&
sortedDir.isDirectory()) {
+ log.info("Reading from provided sorted files directory [{}] (via
system property '{}')",
+ sortedDir.getAbsolutePath(), OAK_INDEXER_SORTED_FILE_PATH);
Review Comment:
The log message might be a bit misleading... here you write "(via system
property x)" but in fact the caller used that system property
(createdSortedStoreFiles()), not this class... What about logging in the caller
about the system property value, and here just "Reading from provided sorted
files directory [{}]"?
This will log twice, but I think that's fine: it is more explicit on what it
is doing.
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java:
##########
@@ -222,26 +235,53 @@ public List<FlatFileStore> buildList(IndexHelper
indexHelper, IndexerSupport ind
return storeList;
}
- private File createdSortedStoreFile() throws IOException,
CompositeException {
+ /**
+ * Returns the existing list of store files if it can read from system
property OAK_INDEXER_SORTED_FILE_PATH which
+ * defines the existing folder where the flat file store files are
present. Will throw an exception if it cannot
+ * read or the path in the system property is not a directory.
+ * If the system property OAK_INDEXER_SORTED_FILE_PATH in undefined, or it
cannot read relevant files it
+ * initializes the flat file store.
+ *
+ * @return list of flat files
+ * @throws IOException
+ * @throws CompositeException
+ */
+ private List<File> createdSortedStoreFiles() throws IOException,
CompositeException {
+ // Check system property defined path
String sortedFilePath =
System.getProperty(OAK_INDEXER_SORTED_FILE_PATH);
Review Comment:
(I have to admit I don't understand why we use a system property, and not a
parameter... but this is not the fault of this patch, so let's keep that for
now.)
--
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]
