Skip to content
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.

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

multithreaded commit() may hang after updating schema #136

Open
tzaeschke opened this issue Jan 20, 2023 · 2 comments · Fixed by #137
Open

multithreaded commit() may hang after updating schema #136

tzaeschke opened this issue Jan 20, 2023 · 2 comments · Fixed by #137
Assignees
Labels

Comments

@tzaeschke
Copy link
Owner

A call to commit() after defining a new database schema may hang indefinitely after if another transaction is open (in the same PMF).

package org.zoodb.test;

import org.zoodb.jdo.ZooJdoHelper;
import org.zoodb.jdo.ZooJdoProperties;
import org.zoodb.tools.ZooHelper;

import javax.jdo.JDOHelper;
import javax.jdo.PersistenceManager;
import javax.jdo.PersistenceManagerFactory;

public class DummyTest {
    public static void main(String[] args) {
        String dbName = "fail.zoo";
        if (ZooHelper.dbExists(dbName)) {
            ZooHelper.removeDb(dbName);
        }
        ZooHelper.createDb(dbName);


        ZooJdoProperties props = new ZooJdoProperties("fail.zoo");
        props.setMultiThreaded(true);
        PersistenceManagerFactory pmf = JDOHelper.getPersistenceManagerFactory(props);
        PersistenceManager pmMain = pmf.getPersistenceManager();
        pmMain.currentTransaction().begin();
        // ZooJdoHelper.schema(pmMain).addClass(Apple.class);      // <-- Workaround
        pmMain.currentTransaction().commit();

        PersistenceManager pmWorker = pmf.getPersistenceManager();

        Runnable r = new Runnable() {
            @Override
            public void run() {
                pmWorker.currentTransaction().begin();
                pmWorker.makePersistent(new Apple());
                pmWorker.currentTransaction().commit();
                pmMain.currentTransaction().begin();
                System.out.print("begin last commit in thread... "); // is reached
                pmMain.currentTransaction().commit();
                System.out.println("done!"); // problem: is not reached
            }
        };
        Thread t = new Thread(r, "Some external thread");
        t.start();
    }
}

and

package org.zoodb.test;

import org.zoodb.api.impl.ZooPC;

public class Apple extends ZooPC {
}

Analysis

Workaround:

Define the schema before opening other transactions.

Background

This happens because the second transaction is (implicitly) creating a schema for Apple in the database. Multithreading should work fine in normal use cases, but schema definition is different. In effect, a schema definition causes all PMF session to reset, however the reset is blocked because the first PM (pmMain) is still active.

Fix?

Causing all transactions to reset after schema change is a kind of pessimistic fail-early approach. Alternatively, Transaction reset could be delayed until a transaction actually tries to read or write to an outdated schema, I am not sure whether that would necessarily be better. It may also impact performance.

TODO

Either the problem should be fixed (=no blocking behavior), or there should be at least a proper error message what is happening.

Another user reported that making the lock reentrant fixes the problem. This is not the preferred solution because the server is designed such that reentrant locks should not be necessary. In the server design, entering a lock twice points to a problem in the server that should be fixed.

@tzaeschke
Copy link
Owner Author

The "hang" has only been fixed when setting DiskAccessOneFile.allowReadConcurrency(true);, however this is generally not recommended (and not an official feature!).

@tzaeschke
Copy link
Owner Author

Fixing the hang properly requires an improved locking mechanism, e.g.

  • either remove the big kernel log
  • or RLOCK on server operations instead of whole TX duration
  • or at least support full READ concurrency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant