Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1012,15 +1012,12 @@ public void exportCas(SourceDocument aDocument, AnnotationSet aSet, OutputStream
{
// Ensure that the CAS is not being re-written and temporarily unavailable while we export
// it, then add this info to a mini-session to ensure that write-access is known
try (var session = CasStorageSession.openNested(true)) {
try (var session = CasStorageSession.openNested()) {
try (var access = new WithExclusiveAccess(aDocument, aSet)) {
session.add(aDocument.getId(), aSet, EXCLUSIVE_WRITE_ACCESS, access.getHolder());
access.ensureRegisteredInSession();

driver.exportCas(aDocument, aSet, aStream);
}
finally {
session.remove(aDocument.getId(), aSet);
}
}
catch (IOException e) {
throw e;
Expand All @@ -1034,17 +1031,14 @@ public void exportCas(SourceDocument aDocument, AnnotationSet aSet, OutputStream
public void importCas(SourceDocument aDocument, AnnotationSet aSet, InputStream aStream)
throws IOException
{
// Ensure that the CAS is not being re-written and temporarily unavailable while we export
// Ensure that the CAS is not being re-written and temporarily unavailable while we import
// it, then add this info to a mini-session to ensure that write-access is known
try (var session = CasStorageSession.openNested(true)) {
try (var session = CasStorageSession.openNested()) {
try (var access = new WithExclusiveAccess(aDocument, aSet)) {
session.add(aDocument.getId(), aSet, EXCLUSIVE_WRITE_ACCESS, access.getHolder());
access.ensureRegisteredInSession();

driver.importCas(aDocument, aSet, aStream);
}
finally {
session.remove(aDocument.getId(), aSet);
}
}
catch (IOException e) {
throw e;
Expand Down Expand Up @@ -1073,9 +1067,9 @@ public void forceActionOnCas(SourceDocument aDocument, AnnotationSet aSet,
{
// Ensure that the CAS is not being re-written and temporarily unavailable while we check
// upgrade it, then add this info to a mini-session to ensure that write-access is known
try (var session = CasStorageSession.openNested(true)) {
try (var session = CasStorageSession.openNested()) {
try (var access = new WithExclusiveAccess(aDocument, aSet)) {
session.add(aDocument.getId(), aSet, EXCLUSIVE_WRITE_ACCESS, access.getHolder());
access.ensureRegisteredInSession();

var cas = aLoader.load(aDocument, aSet);
access.setCas(cas);
Expand All @@ -1086,9 +1080,6 @@ public void forceActionOnCas(SourceDocument aDocument, AnnotationSet aSet,
realWriteCas(aDocument, aSet, cas);
}
}
finally {
session.remove(aDocument.getId(), aSet);
}
}
catch (IOException e) {
throw e;
Expand Down Expand Up @@ -1146,6 +1137,12 @@ private class WithExclusiveAccess
private long documentId;
private AnnotationSet set;

/** The session this access lives in. */
private final CasStorageSession session;

/** Whether this access has been registered in the {@link #session}. */
private boolean registered;

public WithExclusiveAccess(SourceDocument aDocument, AnnotationSet aSet)
throws CasSessionException
{
Expand All @@ -1154,7 +1151,7 @@ public WithExclusiveAccess(SourceDocument aDocument, AnnotationSet aSet)
documentId = aDocument.getId();
set = aSet;

var session = CasStorageSession.get();
session = CasStorageSession.get();

if (!session.hasExclusiveAccess(aDocument, aSet)) {
LOG.trace("CAS storage session [{}]: trying to briefly borrow CAS [{}]@{}",
Expand All @@ -1179,6 +1176,33 @@ public CasKey getKey()
return key;
}

/**
* Registers the exclusive access held by this context in the CAS storage session so that
* operations nested inside the exclusive access scope recognize that write access has
* legitimately been obtained.
* <p>
* Most users of {@link WithExclusiveAccess} do not need this: holding the lock is enough to
* keep the CAS from being re-written while e.g. its metadata is inspected. It is required
* only when the locked scope hands the CAS - or the data backing it - to code that may
* itself try to write.
* <p>
* Does nothing if an enclosing session already holds exclusive access. In that case the
* access is that session's to register and to release, and this context merely operates
* under it - the CAS is locked either way. The registration is removed again when this
* context is {@link #close() closed}.
* <p>
* Calling this method multiple times has no additional effect.
*/
public void ensureRegisteredInSession()
{
if (holder == null || registered) {
return;
}

session.add(documentId, set, EXCLUSIVE_WRITE_ACCESS, holder);
registered = true;
}

public boolean isCasSet()
{
if (holder != null) {
Expand Down Expand Up @@ -1254,6 +1278,11 @@ public void release()
@Override
public void close()
{
if (registered) {
session.remove(documentId, set);
registered = false;
}

if (holder != null) {
LOG.trace("Returning briefly borrowed CAS [{}]@[{}]({})", set, documentName,
documentId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
Expand Down Expand Up @@ -688,6 +690,116 @@ public void returnObject(CasKey aKey, CasHolder aObj)
}
}

@Test
public void testExportCasWorksWhileEnclosingSessionHoldsExclusiveAccess() throws Exception
{
// A caller which already holds exclusive access to a CAS - e.g. the project exporter after
// lazily creating a missing initial CAS - must still be able to export it. Since the
// exclusive access pool is keyed per document, a second borrow on the same thread can never
// be satisfied: it would block until the borrow timeout and then fail. The export must
// therefore recognize the exclusive access held by the enclosing session instead of trying
// to borrow the CAS again.
var shortTimeoutSut = createStorageService(Duration.ofMillis(250));
var doc = makeSourceDocument(20l, 20l, "test");
var set = AnnotationSet.forTest("test");

try (var session = openNested(true)) {
createCasFile(shortTimeoutSut, doc, set, "This is a test");
}

var buffer = new ByteArrayOutputStream();
try (var session = CasStorageSession.open()) {
// Take exclusive access and keep it for the duration of the export
shortTimeoutSut.readCas(doc, set, EXCLUSIVE_WRITE_ACCESS);

shortTimeoutSut.exportCas(doc, set, buffer);
}

assertThat(buffer.size()).isGreaterThan(0);
}

@Test
public void testExportCasTakesExclusiveAccessWhenNobodyElseHoldsIt() throws Exception
{
// The counterpart to the test above: when no enclosing session holds the CAS, the export
// must acquire the exclusive access itself so that the CAS cannot be re-written on disk
// while its bytes are being copied.
var doc = makeSourceDocument(21l, 21l, "test");
var set = AnnotationSet.forTest("test");

try (var session = openNested(true)) {
createCasFile(doc, set, "This is a test");
}

var buffer = new ByteArrayOutputStream();
try (var session = CasStorageSession.open()) {
sut.exportCas(doc, set, buffer);

// The export must not leave the CAS registered in the caller's session
assertThat(session.getManagedState(doc.getId(), set)).isEmpty();
}

assertThat(buffer.size()).isGreaterThan(0);

// The CAS must have been returned to the pool - if it had not, this would block until the
// borrow timeout and then fail
try (var session = CasStorageSession.open()) {
assertThat(sut.readCas(doc, set, EXCLUSIVE_WRITE_ACCESS)).isNotNull();
}
}

@Test
public void testImportCasWorksWhileEnclosingSessionHoldsExclusiveAccess() throws Exception
{
// Same as for the export: a caller which already holds exclusive access must be able to
// import into the CAS without the import trying to borrow it a second time.
var shortTimeoutSut = createStorageService(Duration.ofMillis(250));
var doc = makeSourceDocument(22l, 22l, "test");
var set = AnnotationSet.forTest("test");

var exported = new ByteArrayOutputStream();
try (var session = CasStorageSession.open()) {
createCasFile(shortTimeoutSut, doc, set, "This is a test");
shortTimeoutSut.exportCas(doc, set, exported);
}

try (var session = CasStorageSession.open()) {
// Take exclusive access and keep it for the duration of the import
shortTimeoutSut.readCas(doc, set, EXCLUSIVE_WRITE_ACCESS);

shortTimeoutSut.importCas(doc, set, new ByteArrayInputStream(exported.toByteArray()));

assertThat(shortTimeoutSut.existsCas(doc, set)).isTrue();
}
}

@Test
public void testForceActionOnCasWorksWhenNobodyElseHoldsExclusiveAccess() throws Exception
{
// Unlike the export, forceActionOnCas replaces the CAS in the exclusive access holder. That
// requires it to have borrowed the CAS itself - it cannot replace a CAS which an enclosing
// session registered - so it is only exercised without an enclosing lock here.
var doc = makeSourceDocument(23l, 23l, "test");
var set = AnnotationSet.forTest("test");

try (var session = openNested(true)) {
createCasFile(doc, set, "This is a test");
}

var actionWasApplied = new AtomicBoolean(false);
try (var session = CasStorageSession.open()) {
sut.forceActionOnCas(doc, set, //
(d, s) -> driver.readCas(d, s), //
(d, s, cas) -> actionWasApplied.set(true), //
true);

// The action must not leave the CAS registered in the caller's session
assertThat(session.getManagedState(doc.getId(), set)).isEmpty();
}

assertThat(actionWasApplied).isTrue();
}

@Test
public void testOutOfMemoryDuringExclusiveCasLoadDoesNotStrandCasKey() throws Exception
{
Expand Down
Loading