Skip to content

Issue 52885: Deadlock under heavy server load running post-commit tasks #6584

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Issue 52885: Deadlock under heavy server load running post-commit tasks #6584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 26 additions & 6 deletions api/src/org/labkey/api/data/ContainerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ public class ContainerManager
public static final String HOME_PROJECT_PATH = "/home";
public static final String DEFAULT_SUPPORT_PROJECT_PATH = HOME_PROJECT_PATH + "/support";

// Use double the max count as the size since we cache by both EntityId and Path
private static final Cache<String, Container> CACHE = CacheManager.getStringKeyCache(Constants.getMaxContainers() * 2, CacheManager.DAY, "Containers");
// Use triple the max count as the size since we cache by EntityId, RowId, and Path
private static final Cache<String, Container> CACHE = CacheManager.getStringKeyCache(Constants.getMaxContainers() * 3, CacheManager.DAY, "Containers");
private static final Cache<String, List<String>> CACHE_CHILDREN = CacheManager.getStringKeyCache(Constants.getMaxContainers(), CacheManager.DAY, "Child EntityIds of Containers");
private static final Cache<String, Set<Container>> CACHE_ALL_CHILDREN = CacheManager.getStringKeyCache(Constants.getMaxContainers(), CacheManager.DAY, "Child Container objects of Containers");
private static final ReentrantLock DATABASE_QUERY_LOCK = new ReentrantLockWithName(ContainerManager.class, "DATABASE_QUERY_LOCK");
Expand Down Expand Up @@ -1112,9 +1112,26 @@ private static Map<String, Container> getChildrenMap(Container parent)

public static Container getForRowId(int id)
{
Selector selector = new SqlSelector(CORE.getSchema(), new SQLFragment("SELECT * FROM " + CORE.getTableInfoContainers() + " WHERE RowId = ?", id));
return selector.getObject(Container.class);
}
Container d = getFromCacheId(Integer.toString(id));
if (null != d)
return d;

try (DbScope.Transaction t = ensureTransaction())
{
Container result = new SqlSelector(
CORE.getSchema(),
"SELECT * FROM " + CORE.getTableInfoContainers() + " WHERE RowId = ?",
id).getObject(Container.class);
if (result != null)
{
result = _addToCache(result);
}
// No database changes to commit, but need to decrement the counter
t.commit();

return result;
}
}

public static Container getForId(@NotNull GUID id)
{
Expand Down Expand Up @@ -2076,7 +2093,8 @@ private static String toString(Container c)

private static String toString(Path p)
{
return StringUtils.strip(p.toString(), "/").toLowerCase();
// Differentiate paths from other types of cache keys with the "/" prefix
return "/" + StringUtils.strip(p.toString(), "/").toLowerCase();
}

private static Container _addToCache(Container c)
Expand All @@ -2085,6 +2103,7 @@ private static Container _addToCache(Container c)
"higher level so that we ensure that the container to be inserted still exists and hasn't been deleted";
CACHE.put(toString(c), c);
CACHE.put(c.getId(), c);
CACHE.put(Integer.toString(c.getRowId()), c);
return c;
}

Expand All @@ -2099,6 +2118,7 @@ private static void _removeFromCache(Container c, boolean hierarchyChange)
{
CACHE.remove(toString(c));
CACHE.remove(c.getId());
CACHE.remove(Integer.toString(c.getRowId()));

// Blow away the children caches, which can be outdated even if the hierarchy hasn't changed
// For example, changing the folder type or enabled modules in a parent can impact child workbooks
Expand Down
13 changes: 8 additions & 5 deletions api/src/org/labkey/api/data/DbScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2718,18 +2718,21 @@ public void commit()
conn.commit();
conn.setAutoCommit(true);
LOG.debug("setAutoCommit(true)");
if (null != _closeOnClose)
try { _closeOnClose.close(); } catch (Exception ignore) {}

// Issue 52885 - Use the same DB connection for the commit tasks
CommitTaskOption.POSTCOMMIT.run(this);

// Then finish unwinding the transaction stack
popCurrentTransaction();
}
finally
{
if (null != _closeOnClose)
try { _closeOnClose.close(); } catch (Exception ignore) {}
if (null != conn)
conn.internalClose();
}

popCurrentTransaction();

CommitTaskOption.POSTCOMMIT.run(this);
}
catch (SQLException e)
{
Expand Down