[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/jena/pull/336


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159750664
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java
 ---
@@ -146,25 +162,84 @@ private void _begin(ReadWrite readWrite) {
 }
 
 /** Called transaction start code at most once per transaction. */ 
-private void startTransaction(ReadWrite mode) {
-transactionLock.enterCriticalSection(mode.equals(READ)); // get 
the dataset write lock, if needed.
-transactionType(mode);
+private void startTransaction(TxnType txnType, ReadWrite mode) {
+transactionLock.enterCriticalSection(mode.equals(ReadWrite.READ)); 
// get the dataset write lock, if needed.
+transactionType.set(txnType);
+transactionMode(mode);
 isInTransaction(true);
 }
 
 /** Called transaction ending code at most once per transaction. */ 
 private void finishTransaction() {
 isInTransaction.remove();
 transactionType.remove();
+transactionMode.remove();
 version.remove();
 transactionLock.leaveCriticalSection();
 }
  
+@Override
+public boolean promote() {
+if (!isInTransaction())
+throw new JenaTransactionException("Tried to promote outside a 
transaction!");
+if ( transactionMode().equals(ReadWrite.WRITE) )
+return true;
+
+boolean readCommitted;
+// Initial state
+switch(transactionType.get()) {
+case WRITE :
+return true;
+case READ :
+throw new JenaTransactionException("Tried to promote READ 
transaction");
+case READ_COMMITTED_PROMOTE :
+readCommitted = true;
+case READ_PROMOTE :
+readCommitted = false;
+// Maybe!
+break;
+default:
+throw new NullPointerException();
+}
+
+try {
+_promote(readCommitted);
+return true;
+} catch (JenaTransactionException ex) {
+return false ;
+}
+}
+
+private void _promote(boolean readCommited) {
+//System.err.printf("Promote: version=%d generation=%d\n", 
version.get() , generation.get()) ;
+
+// Outside lock.
+if ( ! readCommited && version.get() != generation.get() )  {
--- End diff --

I'll take that as an "approved" :-) and merge it.

This PR is big so we can keep reviewing after the merge if people want to 
make further comments.


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159748796
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java
 ---
@@ -146,25 +162,84 @@ private void _begin(ReadWrite readWrite) {
 }
 
 /** Called transaction start code at most once per transaction. */ 
-private void startTransaction(ReadWrite mode) {
-transactionLock.enterCriticalSection(mode.equals(READ)); // get 
the dataset write lock, if needed.
-transactionType(mode);
+private void startTransaction(TxnType txnType, ReadWrite mode) {
+transactionLock.enterCriticalSection(mode.equals(ReadWrite.READ)); 
// get the dataset write lock, if needed.
+transactionType.set(txnType);
+transactionMode(mode);
 isInTransaction(true);
 }
 
 /** Called transaction ending code at most once per transaction. */ 
 private void finishTransaction() {
 isInTransaction.remove();
 transactionType.remove();
+transactionMode.remove();
 version.remove();
 transactionLock.leaveCriticalSection();
 }
  
+@Override
+public boolean promote() {
+if (!isInTransaction())
+throw new JenaTransactionException("Tried to promote outside a 
transaction!");
+if ( transactionMode().equals(ReadWrite.WRITE) )
+return true;
+
+boolean readCommitted;
+// Initial state
+switch(transactionType.get()) {
+case WRITE :
+return true;
+case READ :
+throw new JenaTransactionException("Tried to promote READ 
transaction");
+case READ_COMMITTED_PROMOTE :
+readCommitted = true;
+case READ_PROMOTE :
+readCommitted = false;
+// Maybe!
+break;
+default:
+throw new NullPointerException();
+}
+
+try {
+_promote(readCommitted);
+return true;
+} catch (JenaTransactionException ex) {
+return false ;
+}
+}
+
+private void _promote(boolean readCommited) {
+//System.err.printf("Promote: version=%d generation=%d\n", 
version.get() , generation.get()) ;
+
+// Outside lock.
+if ( ! readCommited && version.get() != generation.get() )  {
--- End diff --

Yeah, I knew this stuff was here but I didn't read it carefully when you 
put it in months ago because we weren't turning it on. Okay, I'm :+1: . Let 'er 
rip!


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159746359
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java
 ---
@@ -146,25 +162,84 @@ private void _begin(ReadWrite readWrite) {
 }
 
 /** Called transaction start code at most once per transaction. */ 
-private void startTransaction(ReadWrite mode) {
-transactionLock.enterCriticalSection(mode.equals(READ)); // get 
the dataset write lock, if needed.
-transactionType(mode);
+private void startTransaction(TxnType txnType, ReadWrite mode) {
+transactionLock.enterCriticalSection(mode.equals(ReadWrite.READ)); 
// get the dataset write lock, if needed.
+transactionType.set(txnType);
+transactionMode(mode);
 isInTransaction(true);
 }
 
 /** Called transaction ending code at most once per transaction. */ 
 private void finishTransaction() {
 isInTransaction.remove();
 transactionType.remove();
+transactionMode.remove();
 version.remove();
 transactionLock.leaveCriticalSection();
 }
  
+@Override
+public boolean promote() {
+if (!isInTransaction())
+throw new JenaTransactionException("Tried to promote outside a 
transaction!");
+if ( transactionMode().equals(ReadWrite.WRITE) )
+return true;
+
+boolean readCommitted;
+// Initial state
+switch(transactionType.get()) {
+case WRITE :
+return true;
+case READ :
+throw new JenaTransactionException("Tried to promote READ 
transaction");
+case READ_COMMITTED_PROMOTE :
+readCommitted = true;
+case READ_PROMOTE :
+readCommitted = false;
+// Maybe!
+break;
+default:
+throw new NullPointerException();
+}
+
+try {
+_promote(readCommitted);
+return true;
+} catch (JenaTransactionException ex) {
+return false ;
+}
+}
+
+private void _promote(boolean readCommited) {
+//System.err.printf("Promote: version=%d generation=%d\n", 
version.get() , generation.get()) ;
+
+// Outside lock.
+if ( ! readCommited && version.get() != generation.get() )  {
--- End diff --

Yes- that's what happens. This part of the code isn't new - only moved. The 
check is done twice, once before the lock, once again after to avoid 
unnecessary `transactionLock` taking.

It's the same algorithm in TDB and TDB2.  TDB and TDB2 have explicit 
`Transaction` objects that carry the start version. The shared tests cover all 
cases.



---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159729634
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java
 ---
@@ -146,25 +162,84 @@ private void _begin(ReadWrite readWrite) {
 }
 
 /** Called transaction start code at most once per transaction. */ 
-private void startTransaction(ReadWrite mode) {
-transactionLock.enterCriticalSection(mode.equals(READ)); // get 
the dataset write lock, if needed.
-transactionType(mode);
+private void startTransaction(TxnType txnType, ReadWrite mode) {
+transactionLock.enterCriticalSection(mode.equals(ReadWrite.READ)); 
// get the dataset write lock, if needed.
+transactionType.set(txnType);
+transactionMode(mode);
 isInTransaction(true);
 }
 
 /** Called transaction ending code at most once per transaction. */ 
 private void finishTransaction() {
 isInTransaction.remove();
 transactionType.remove();
+transactionMode.remove();
 version.remove();
 transactionLock.leaveCriticalSection();
 }
  
+@Override
+public boolean promote() {
+if (!isInTransaction())
+throw new JenaTransactionException("Tried to promote outside a 
transaction!");
+if ( transactionMode().equals(ReadWrite.WRITE) )
+return true;
+
+boolean readCommitted;
+// Initial state
+switch(transactionType.get()) {
+case WRITE :
+return true;
+case READ :
+throw new JenaTransactionException("Tried to promote READ 
transaction");
+case READ_COMMITTED_PROMOTE :
+readCommitted = true;
+case READ_PROMOTE :
+readCommitted = false;
+// Maybe!
+break;
+default:
+throw new NullPointerException();
+}
+
+try {
+_promote(readCommitted);
+return true;
+} catch (JenaTransactionException ex) {
+return false ;
+}
+}
+
+private void _promote(boolean readCommited) {
+//System.err.printf("Promote: version=%d generation=%d\n", 
version.get() , generation.get()) ;
+
+// Outside lock.
+if ( ! readCommited && version.get() != generation.get() )  {
--- End diff --

Just checking that I understand the algo here...

1. Every transaction pulls a version number from the current generation 
number.
2. A commit increments the generation number.
3. When a transaction attempts to promote, it checks to see that the 
generation hasn't incremented out from under it (indicating that some other 
transaction committed since this one began). If that happened, this transaction 
will throw an exception as shown below.
4. Otherwise, it's okay to promote.

So two questions:
1. Is that understanding accurate?
2. Is that essentially how this is implemented in other dataset impls, or 
do I need to go read them as well?


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159162581
  
--- Diff: jena-arq/src/main/java/org/apache/jena/query/TxnType.java ---
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query;
+
+import java.util.Objects;
+
+import org.apache.jena.sparql.JenaTransactionException;
+
+public enum TxnType {
+/** Transaction mode:
+ * 
+ * {@code WRITE}: this gaurantees a WRITE will complete if {@code 
commit()} is
--- End diff --

typo: `gaurantees` -> `guarantees`


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/336#discussion_r159162566
  
--- Diff: jena-arq/src/main/java/org/apache/jena/query/TxnType.java ---
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query;
+
+import java.util.Objects;
+
+import org.apache.jena.sparql.JenaTransactionException;
+
+public enum TxnType {
+/** Transaction mode:
+ * 
+ * {@code WRITE}: this gaurantees a WRITE will complete if {@code 
commit()} is
+ * called. The same as {@code begin(ReadWrite.WRITE)}.
+ * 
+ * {@code READ}: the transaction can not promote to WRITE,ensuring 
read-only
+ * access to the data. The same as {@code begin(ReadWrite.READ)}.
+ * 
+ * {@code READ_PROMOTE}: the transaction will go from "read" to 
"write" if the
+ * dataset has not been modified but if it has, the promotion fails 
with
+ * exception.
+ * 
+ * {@code READ_COMMITTED_PROMOTE}: Use this with care. The 
promotion will succeed but 
+ * changes from oher transactions become visible.
+ * 
+ * 
+ * Read committed: at the point transaction attempts promotion from 
"read" to
+ * "write", the sytem checks if the datset has chnage since the 
trsnaction started
+ * (called {@code begin}). If {@code READ_PROMOTE}, the dataset must 
not have
+ * changed; if {@code READ_COMMITTED_PROMOTE} anyh intermediate 
changes are
--- End diff --

typo : `anyh` -> `any`


---

[GitHub] jena pull request #336: JENA-1458: Transaction promotion

[afs]

GitHub user afs opened a pull request:

https://github.com/apache/jena/pull/336

JENA-1458: Transaction promotion

This PR provides promotion  (read mode goes to write mode as needed) for 
TIM, TDB1 and TDB2.

Transactions have a type (new) and a current mode.

* TxnMode (enum `ReadWrite` - name retained for compatibility)
* `TxnType` - `WRITE`, `READ`, `READ_PROMOTE`, `READ_COMMITTED_PROMOTE`

Transactions can go from "read" to "write", if their initial setup allows 
it, when an update operation is attempted (implicit promotion) or if the app 
asks for the transaction to be promoted (explicit promotion). 

`Transactional` and hence the DatasetGraph/Dataset interfaces get new 
operations:

WRITE and READ are the same as the previous use of ReadWrite for W and R 
transactions.

The two different types of promote differ in whether strict transaction 
isolation is
maintained (`READ_PROMOTE`) and no changes from other writers
are seen during the transaction, or (`READ_COMMITTED_PROMOTE`) whether at 
the point of promotion, changes become visible.  
 
`READ_PROMOTE` can fail (implicit promote - transaction aborts; explicit 
promote, `promote()` returns false) whereas `READ_COMMITTED_PROMOTE` does not 
fail - it's like ending a READ transaction and starting a fresh WRITE 
transaction; changes from other writers can be seen across the change.
 
The new operations are:
```
public void begin()
public void begin(TxnType type)
public boolean promote()
public ReadWrite transactionMode();
public TxnMode transactionType();
```
and `public void begin(ReadWrite mode)` remains with the same semantics as 
before. No application needs to change.

`begin()` is `begin(TxnType.READ_PROMOTE)`, which can cause a system abort.

This is a choice - it could be `begin(TxnType.READ_COMMITTED_PROMOTE)` 
which does not cause system aborts but any reads before the promote point are 
not no longer guaranteed to be the state of the data (the transaction is "read 
committed", not strong isolation properties) leading to possible inconsistent 
data update.

Graph transactions handlers call `begin()`.

To do:

* Update `Txn`
* More checking



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/afs/jena promotion

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jena/pull/336.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #336


commit 74808450e635418cca758a564288fc80994b3ad6
Author: Andy Seaborne 
Date:   2017-12-29T18:56:57Z

Remove duplicate ThreadTxn from DBOE.

commit 143ac13b69a26b91d0f6f97991794231c4edd0cd
Author: Andy Seaborne 
Date:   2017-12-30T12:30:35Z

JENA-1458: Promotion API and convert TIM

commit ee7507df9fe24abcf076e449807dc8d056699392
Author: Andy Seaborne 
Date:   2017-12-30T22:57:06Z

JENA-1458: Use TxnType, replacing ReadWrite mode

commit 64f6357c25494a961f77db670cbe5b2d4cc35eab
Author: Andy Seaborne 
Date:   2017-12-30T22:57:46Z

JENA-1458: Promotion API integration for TDB

commit d6770eaf62f13d60bc5f414cfa6b2f3a50fca445
Author: Andy Seaborne 
Date:   2017-12-31T17:15:04Z

JENA-1458: Promotion API integration for TDB2

commit 55983cdae25a2a9d23b695486e4488f76fd94786
Author: Andy Seaborne 
Date:   2017-12-31T20:49:18Z

Fix to work with tests. Cleanup.




---