Plain begin - read transactions promoting to write transactions.

[afs]

This PR is for discussion and is not yet ready for merging.

It adds to TDB the ability for a read transaction to promote to a write transaction.

Currently (to avoid general API changes outside TDB) this happens automatically in DatasetGraphTransaction.getW().

It needs to be enabled with DatasetGraphTransaction.promotion = true

If this is useful, we can add new modes to ReadWrite and/or add begin() (no args).

There is a big design point: at the point at which a transaction becomes a writer their are two choices as to what to do if another write transaction happened between this one starting and this one promoting.

1. The transaction can now see changes made by the other writer. A limited form of "read committed" behaviour. Downside: results from actions during the read phase may be invalidated.
2. The transaction is blocked from promoting. This is purer (results are not invalidated; an error is returned) but the code must deal with it.

Having it as a choice is possible - one should be the default for begin().

[afs]

Example:

    public static void dsgTxn() {
        DatasetGraphTransaction.promotion = true ;
        DatasetGraph dsg = TDBFactory.createDatasetGraph() ;

        Quad q1 = SSE.parseQuad("(_ :s :p1 1)") ;

        dsg.begin(ReadWrite.READ); 
        // Start R->W here
        dsg.add(q1) ;
        dsg.commit();
        dsg.end() ;

        Txn.execRead(dsg, ()->{
            RDFDataMgr.write(System.out, dsg.getDefaultGraph(), Lang.NT);
        });

        System.out.println("DONE") ;
    }

NB Current Txn does not work for promotion - it needs to commit(), not just end() a read transaction.

[ajs6f]

Hm, just a first gut reaction about that example API: I would not want to initiate a promotion just by signalling system-wide and then writing. I'd much rather some positive step be taken on that particular dataset, to record very clear intent. I'm guessing that although it's really not good style, there are folks out there depending on the system to throw an exception for writing in a read txn as part of their logic. New modes for ReadWrite sounds like a nice solution to this. (e.g. PromotableRead?)

[ajs6f]

At first look, I would prefer the second option for dealing with intervening writes. It's blunter, but it seems a good deal easier for application writers to reason about. But maybe I'm not giving enough weight to the value of greater concurrency.

[afs]

DatasetGraphTransaction.promotion = true is only needed for this illustrative PR.

It means for exploration, so you can swap modes.

It will removed when the code is real.

[afs]

I'd like to get his code into master even if it is switched off. If we are comfortable with it the general way to go, it's easier to have it live. If not enabled, the code should make no difference to current behaviour.

[ajs6f]

Sorry, I let this go without replying. No, the DatasetGraphTransaction.promotion isn't really my concern. It's the fact that you start a read transaction on a dataset, then try to write, and instead of getting an exception, you get promoted. That seems a bit surprising and dangerous to me. I'm not trying to block this, but wouldn't it be possible to add a new value for ReadWrite now instead of after the fact? Old code could just ignore it and new code could check for it to see if promotion is enabled for the given transaction.

[afs]

The proposal is not to adopt every detail of this PR; it's to include but not activate it.

I think we should try to add the feature to all the transactional implementations (TDB, TIM, MRSW - not SDB) together. Having this in the code base, not activated by default, is a step on that road.

Automatically allowing writes is normal behaviour for JDBC which has implicit begin().

Exceptions don't work well. Some transaction-unaware library code might make the update causing the transaction to promote. An exception will crash out of the middle of the library code.

The name ReadWrite may not be a good choice if start adding more modes. Also, we need to check how old code ignores a new value.

[afs]

Eventually, a begin() would start a transaction in a way that is promotable. This fits the expectation of JDBC (it's even implicit there) and help bring the existing graph-level TransactionHandler into line (an existing API so the barrier to change is higher, albeit an API that isn't likely to be used much as it does not automatically fit with dataset transactions).

[afs]

Proposal: the default behaviour is "no read commited" for begin() (behavior 2 in the description) with an option to call setReadCommited (or some such name). And/or a readCurrent which effectively resets a read transaction to see the current (up to last commit) state.

[ajs6f]

I like the proposal, basically, although I suspect it might be a little tricky to do the original suggestion of "limited read committed" in current TIM. Adding readCurrent would make seem to make that easier. But as long as the API seems good, we'll get it done.

Do you think it would be useful to offer a "block until promoted" option? Or is that basically covered by "end your current xaction and open a new WRITE xaction"?

[afs]

(mis-closed - another "close next to comment" incident)

[afs]

See JENA-1223 for overall discussion of API issues and to record changes in subsystems.

[afs]

Squash development commits to a few important ones.

[afs]

After that fix, the tests have reasonable coverage of all modes so this is ready to merge from my point-of-view.

[afs]

To track whether a promotion is possible for fully serialized, the code now checks a version number. The necessary information is available elsewhere (whether a new transaction would see the same dataset as one wanting promotion) but it's crypt and in TIM, that's in the indexes, not the transaction control in the dataset.

Keeping the version is also a nice statistic to keep.

The version moves on one when a transaction commits.

That leaves one case - what to do when a writer is already active. That writer may commit later (and hence the promotion is not possible) to abort (the promotion may be possible if no other writer comes and goes). The current refuses promotion if there is an active writer on the basis that most writers commit, and abort is unusual. Blocking maybe possible but leads to issues of what happens when another reader that tries to promote at about the same time and of a long running writer (e.g. bulk upload) causing the reader to block for a long time.

[ajs6f]

So ideally, it would be another writer committing that would throw a switch to refuse promotion to extant transactions? Instead of another writer simply existing?

[afs]

Yes.

[ajs6f]

I guess I'm confused as to why one can't determine (from the version numbers) whether another transaction has committed? If you know a transaction's version number and you know the transaction manager's version number... I think I'm surely missing something about how these classes work, since I only worked with the simpler "thread-is-transaction" thinking in TIM.

[afs]

Version number only tell you about the past (see TransactionManager.notifyCommit where the version number increases - this is as the writer finishes up).

When a writer is active and has not committed or aborted. So how do you wait for the active writer to decide which way it's going to go? (what would be block on to wait for it?)

[ajs6f]

Oh, okay, I get it now. And even if you could block, as you wrote above, that could be a really long time in some cases. Do you want me to write something for TIM to bring a version number to the dataset? I don't think that would be too hard. (Famous last words.)

[afs]

Let's get this into the codebase first (inactive). JENA-1222 and JENA-1224 are both changes to TDB's TransactionManager and juggling multiple branches of nearby code changes is not nice.

I've tried to write a test for snapshot isolation promotion but it relies on a thread pause which elsewhere has been unstable on a heavily loaded CI server (TestTransPromote.promote_clash_active_writer). At the moment, there is no special code for the "active writer then abort" case. It may be possible but if it looks like a major change to TDB1, then it may not be worth and may be a sign that the specific feature is generally something that warps the rest of the system.

Until we get the contract details settled, changing TIM seems premature.

[ajs6f]

Okay, I'll hang tight.