Skip to content

MCR-3695 MCRMetadataManager: per-object locking for write methods #2929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sebhofmann wants to merge 3 commits into
base: main
Choose a base branch
from
Open

MCR-3695 MCRMetadataManager: per-object locking for write methods #2929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58ee834
MCR-3695 MCRMetadataManager: per-object locking for write methods
sebhofmann Apr 29, 2026
57d76f2
MCR-3695 MCRMetadataManagerLockTest: drop empty try body (PMD)
sebhofmann Apr 29, 2026
328de9a
MCR-3695 MCRObjectLock: make close() idempotent
sebhofmann Apr 30, 2026
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
230 changes: 164 additions & 66 deletions mycore-base/src/main/java/org/mycore/datamodel/metadata/MCRMetadataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -81,6 +85,17 @@ public final class MCRMetadataManager {

public static final String DEFAULT_IMPLEMENTATION_KEY = "Default";

/**
* Property key for the per-object lock acquisition timeout (in seconds) used by
* {@link #lock(MCRObjectID)}. When the timeout elapses without acquiring the lock,
* an {@link MCRPersistenceException} is thrown.
*/
public static final String LOCK_TIMEOUT_PROPERTY = "MCR.Metadata.Manager.LockTimeoutSeconds";

private static final int DEFAULT_LOCK_TIMEOUT_SECONDS = 60;

private static final ConcurrentMap<MCRObjectID, LockEntry> LOCKS = new ConcurrentHashMap<>();

private MCRMetadataManager() {
}

Expand All @@ -105,19 +120,20 @@ public static void create(final MCRDerivate mcrDerivate) throws MCRPersistenceEx

derivateId = assignNewIdIfNecessary(mcrDerivate);

validateDerivate(mcrDerivate, derivateId);
try (MCRObjectLock ignored = lock(derivateId)) {
validateDerivate(mcrDerivate, derivateId);

final MCRObjectID objectId = getObjectIDFromDerivate(mcrDerivate);
checkWritePermission(objectId, derivateId);
final MCRObjectID objectId = getObjectIDFromDerivate(mcrDerivate);
checkWritePermission(objectId, derivateId);

byte[] objectBackup = retrieveObjectBackup(objectId);
byte[] objectBackup = retrieveObjectBackup(objectId);

setDerivateMetadata(mcrDerivate);
setDerivateMetadata(mcrDerivate);

fireEvent(mcrDerivate, null, MCREvent.EventType.CREATE);

createDataInIFS(mcrDerivate, derivateId, objectId, objectBackup);
fireEvent(mcrDerivate, null, MCREvent.EventType.CREATE);

createDataInIFS(mcrDerivate, derivateId, objectId, objectBackup);
}
}

private static MCRObjectID assignNewIdIfNecessary(MCRDerivate mcrDerivate) {
Expand Down Expand Up @@ -266,17 +282,19 @@ public static void create(final MCRObject mcrObject) throws MCRPersistenceExcept
MCRObjectID objectId = Objects.requireNonNull(mcrObject.getId(), "ObjectID must not be null");

checkCreatePrivilege(objectId);
// exist the object?
if (exists(objectId)) {
throw new MCRPersistenceException("The object " + objectId + " already exists, nothing done.");
}
try (MCRObjectLock ignored = lock(objectId)) {
// exist the object?
if (exists(objectId)) {
throw new MCRPersistenceException("The object " + objectId + " already exists, nothing done.");
}

normalizeObject(mcrObject);
normalizeObject(mcrObject);

validateObject(mcrObject);
validateObject(mcrObject);

// handle events
fireEvent(mcrObject, null, MCREvent.EventType.CREATE);
// handle events
fireEvent(mcrObject, null, MCREvent.EventType.CREATE);
}
}

public static void checkCreatePrivilege(MCRObjectID objectId) throws MCRAccessException {
Expand Down Expand Up @@ -304,24 +322,26 @@ public static void delete(final MCRDerivate mcrDerivate) throws MCRPersistenceEx
if (!MCRAccessManager.checkDerivateContentPermission(id, PERMISSION_DELETE)) {
throw new MCRMissingPermissionException("Delete derivate", id.toString(), PERMISSION_DELETE);
}
// mark for deletion
MCRMarkManager.getInstance().mark(id, Operation.DELETE);
try (MCRObjectLock ignored = lock(id)) {
// mark for deletion
MCRMarkManager.getInstance().mark(id, Operation.DELETE);

// delete data from IFS
if (mcrDerivate.getDerivate().getInternals() != null) {
try {
deleteDerivate(id.toString());
LOGGER.info("IFS entries for MCRDerivate {} are deleted.", id);
} catch (final Exception e) {
throw new MCRPersistenceException("Error while delete MCRDerivate " + id + " in IFS", e);
// delete data from IFS
if (mcrDerivate.getDerivate().getInternals() != null) {
try {
deleteDerivate(id.toString());
LOGGER.info("IFS entries for MCRDerivate {} are deleted.", id);
} catch (final Exception e) {
throw new MCRPersistenceException("Error while delete MCRDerivate " + id + " in IFS", e);
}
}
}

// handle events
fireEvent(mcrDerivate, null, MCREvent.EventType.DELETE);
// handle events
fireEvent(mcrDerivate, null, MCREvent.EventType.DELETE);

// remove mark
MCRMarkManager.getInstance().remove(id);
// remove mark
MCRMarkManager.getInstance().remove(id);
}
}

/**
Expand Down Expand Up @@ -379,27 +399,31 @@ private static void delete(final MCRObjectID id, MCRObject mcrObject)

checkDeletePermission(id);

checkForActiveLinks(id);
try (MCRObjectLock ignored = lock(id)) {
checkForActiveLinks(id);

markForDeletion(id);
markForDeletion(id);

removeAllChildren(id);
removeAllChildren(id);

removeAllDerivates(id);
removeAllDerivates(id);

if (mcrObject == null) {
try {
mcrObject = retrieveMCRObject(id);
} catch (MCRPersistenceException e) {
LOGGER.error(() -> "Error while deleting " + id + ". Unable to retrieve MCRObject from disk. Create " +
"empty mycoreobject for event handling as fallback.", e);
mcrObject = new MCRObject();
mcrObject.setId(id);
if (mcrObject == null) {
try {
mcrObject = retrieveMCRObject(id);
} catch (MCRPersistenceException e) {
LOGGER.error(
() -> "Error while deleting " + id + ". Unable to retrieve MCRObject from disk. Create " +
"empty mycoreobject for event handling as fallback.",
e);
mcrObject = new MCRObject();
mcrObject.setId(id);
}
}
}
fireEvent(mcrObject, null, MCREvent.EventType.DELETE);
fireEvent(mcrObject, null, MCREvent.EventType.DELETE);

removeMark(id);
removeMark(id);
}
}

private static void removeAllChildren(MCRObjectID id)
Expand Down Expand Up @@ -659,21 +683,22 @@ public static void update(final MCRDerivate mcrDerivate) throws MCRPersistenceEx
return;
}

if (!exists(derivateId)) {
create(mcrDerivate);
return;
}

checkUpdatePermission(derivateId);
try (MCRObjectLock ignored = lock(derivateId)) {
if (!exists(derivateId)) {
create(mcrDerivate);
return;
}

Path fileSourceDirectory = handleFileSourceDirectory(mcrDerivate);
checkUpdatePermission(derivateId);

MCRDerivate old = retrieveMCRDerivate(derivateId);
Path fileSourceDirectory = handleFileSourceDirectory(mcrDerivate);

updateDerivate(mcrDerivate, old);
MCRDerivate old = retrieveMCRDerivate(derivateId);

updateIFS(fileSourceDirectory, derivateId);
updateDerivate(mcrDerivate, old);

updateIFS(fileSourceDirectory, derivateId);
}
}

private static Path handleFileSourceDirectory(MCRDerivate mcrDerivate) {
Expand Down Expand Up @@ -730,21 +755,23 @@ public static void update(final MCRObject mcrObject) throws MCRPersistenceExcept
return;
}

if (!exists(id)) {
create(mcrObject);
return;
}
try (MCRObjectLock ignored = lock(id)) {
if (!exists(id)) {
create(mcrObject);
return;
}

checkUpdatePermission(id);
normalizeObject(mcrObject);
validateObject(mcrObject);
checkUpdatePermission(id);
normalizeObject(mcrObject);
validateObject(mcrObject);

MCRObject old = retrieveMCRObject(id);
MCRObject old = retrieveMCRObject(id);

checkModificationDates(mcrObject, old);
retainCreatedDateAndFlags(mcrObject, old);
checkModificationDates(mcrObject, old);
retainCreatedDateAndFlags(mcrObject, old);

fireUpdateEvent(mcrObject);
fireUpdateEvent(mcrObject);
}
}

/**
Expand Down Expand Up @@ -909,4 +936,75 @@ public static MCRObjectID getObjectId(MCRObjectID derivateID) {
return null;
});
}

/**
* Acquires the per-object lock for {@code id}, blocking up to the timeout configured by
* {@link #LOCK_TIMEOUT_PROPERTY} (default 60s). The returned {@link MCRObjectLock} must be
* closed (use try-with-resources). Reentrant: the same thread may acquire the same id any
* number of times; only the outermost {@code close()} releases the lock for other threads.
*
* @param id the object id to lock
* @return an AutoCloseable lock handle
* @throws MCRPersistenceException if the timeout elapses or the wait is interrupted
*/
public static MCRObjectLock lock(MCRObjectID id) {
LockEntry entry = LOCKS.compute(id, (k, v) -> {
if (v == null) {
v = new LockEntry();
}
v.refCount++;
return v;
});
boolean acquired = false;
try {
acquired = entry.lock.tryLock(getLockTimeoutSeconds(), TimeUnit.SECONDS);
if (!acquired) {
throw new MCRPersistenceException("Lock timeout for " + id);
}
return new MCRObjectLock(id);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new MCRPersistenceException("Lock interrupted for " + id, e);
} finally {
if (!acquired) {
LOCKS.compute(id, (k, v) -> --v.refCount == 0 ? null : v);
}
}
}

private static int getLockTimeoutSeconds() {
return MCRConfiguration2.getInt(LOCK_TIMEOUT_PROPERTY).orElse(DEFAULT_LOCK_TIMEOUT_SECONDS);
}

private static final class LockEntry {
final ReentrantLock lock = new ReentrantLock();
int refCount; // guarded per key by ConcurrentHashMap.compute on LOCKS
}

/**
* AutoCloseable handle returned by {@link MCRMetadataManager#lock(MCRObjectID)}.
* Release by calling {@link #close()}, typically via try-with-resources.
*/
public static final class MCRObjectLock implements AutoCloseable {

private final MCRObjectID id;

private boolean closed;

MCRObjectLock(MCRObjectID id) {
this.id = id;
}

@Override
public void close() {
if (closed) {
return;
}
closed = true;
LOCKS.compute(id, (k, v) -> {
v.lock.unlock();
return --v.refCount == 0 ? null : v;
});
}
}
}
4 changes: 4 additions & 0 deletions mycore-base/src/main/resources/config/mycore.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ MCR.Access.Facts.Condition.category=org.mycore.access.facts.condition.fact.MCRCa
# Which metadata manager to use (dictates the available stores)
MCR.Metadata.Manager.Class=org.mycore.datamodel.common.MCRDefaultXMLMetadataManager

# Timeout in seconds for acquiring the per-object write lock used by MCRMetadataManager.
# A timeout throws MCRPersistenceException; the caller can retry, surface, or abort.
MCR.Metadata.Manager.LockTimeoutSeconds=60

# Metadata store for derivate XML
MCR.IFS2.Store.derivate.Class=org.mycore.datamodel.ifs2.MCRVersioningMetadataStore
MCR.IFS2.Store.derivate.SlotLayout=4-2-2
Expand Down
Loading
Loading