[
https://issues.apache.org/jira/browse/OAK-9914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17598456#comment-17598456
]
Julian Sedding commented on OAK-9914:
-------------------------------------
I think the proposed fix is incorrect. A simpler fix changes
{{AzureArchiveManager#open()}} to return {{null}} instead of throwing an
exception, and reduces the log level of the read-only recovery messages from
WARN to INFO. I explain my rationale below.
Note that the file-based segment store (i.e. TarPersistence) also creates a
file ending in {{.ro.bak}} when a read-only store is opened and the last tar
file does not contain an index (yet).
I believe that the index for an open file in the rw store is maintained
in-memory and only written when the file is closed. It can of course happen
that an application crashes/is killed and a tar file is left without an index,
which warrants a WARN log message about recovery.
For the read-only case, I would argue that the log message should be at most at
INFO level, which would be a simple fix in {{TarReader}}. Also, the javadocs of
{{SegmentArchiveManager#open()}} say the following regarding the return value:
"the archive reader or null if the archive doesn't exist or doesn't have a
valid index". Therefore, I would argue that the {{AzureArchiveManager}} mustn't
throw an exception, but should just return {{null}}.
Test case using TarPersistence for comparison, also look at the log messages:
{code:java}
@Test
public void testReadOnlyRecoveryTar() throws
InvalidFileStoreVersionException, IOException, CommitFailedException {
final File directory = folder.newFolder();
FileStore rwFileStore =
FileStoreBuilder.fileStoreBuilder(directory).build();
SegmentNodeStore segmentNodeStore =
SegmentNodeStoreBuilders.builder(rwFileStore).build();
NodeBuilder builder = segmentNodeStore.getRoot().builder();
builder.setProperty("foo", "bar");
segmentNodeStore.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
rwFileStore.flush();
assertTrue(new File(directory, "data00000a.tar").isFile());
assertFalse(new File(directory, "data00000a.tar.ro.bak").exists());
// create read-only FS
ReadOnlyFileStore roFileStore =
FileStoreBuilder.fileStoreBuilder(directory).buildReadOnly();
roFileStore.close();
rwFileStore.close();
assertTrue(new File(directory, "data00000a.tar").isFile());
assertFalse(new File(directory, "data00000a.tar.ro.bak").exists()); //
fails and the log contains warn message regarding recovery
}
{code}
> Starting Oak with Azure persistence in read-only mode while another Oak
> process is running will initiate repo recovery
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: OAK-9914
> URL: https://issues.apache.org/jira/browse/OAK-9914
> Project: Jackrabbit Oak
> Issue Type: New Feature
> Components: segment-azure
> Affects Versions: 1.44.0
> Reporter: Miroslav Smiljanic
> Assignee: Miroslav Smiljanic
> Priority: Major
> Attachments: AzureArchiveManager.patch, OAK-9914_test.patch
>
>
> The sequence of events:
> # Oak process with read/write file store utilizing Azure persistence is
> already running
> # New Oak process is starting up, with read-only file store using Azure
> persistence and the same storage account and container like the previous Oak
> process
> ## New Oak process starts recovery procedure for the last tar directory that
> is not closed
> Scenario presented in the attached test case:
> [^OAK-9914_test.patch]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
