[MDSAL-775] Do not allow cross-datastore transactions Created: 10/Oct/22  Updated: 08/Nov/22  Resolved: 07/Nov/22

Status: Resolved
Project: mdsal
Component/s: DOM runtime
Affects Version/s: None
Fix Version/s: 11.0.0

Type: Task Priority: Medium
Reporter: Robert Varga Assignee: Ruslan Kashapov
Resolution: Done Votes: 0
Labels: pt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Issue split
split to CONTROLLER-2055 Do not allow cross-datastore transact... Resolved

 Description   

The ability to access multiple datastores in a single transaction is something we supported on a best-effort basis, but there is no concept of cross-datastore atomicity.

As such, no downstreams should be using this facility – remove this ability, forcing users to have a separate transaction for each datastore.



 Comments   
Comment by Ruslan Kashapov [ 13/Oct/22 ]

there is a limitation to what can be done as solution for current task
due to the way the datastore being addressed using current API.

in order to operate with data from a specific datastore the composite transaction 
object should be created first, then only datastore will be addressed as an
argument of read/write operation like below

final WriteTransaction tx = dataBroker.newWriteOnlyTransaction();
tx.merge(LogicalDatastoreType.CONFIGURATION, DATA_IID, data); 

it means backing transactions for all the datastores require to be instantiated
before the actual datastore is mentioned.

the composite transaction is being created by dataBroker from the set of datastores
provided on databroker initialization. it means in order to have single backing transaction
within composite one the dataBroker have to be always initialized with single datastore.

necessity to have separate databroker instance for each datastore isn't suitable
with current usage of databroker, it means no backward compatibility.

#1

the compromise solution could be logical restriction within transaction itself by controlling
the datastore type to match the first requested one like shown below

final WriteTransaction tx = dataBroker.newWriteOnlyTransaction();
tx.merge(LogicalDatastoreType.CONFIGURATION, DATA_IID, data); // first operation, datastore type registered 
tx.merge(LogicalDatastoreType.CONFIGURATION, DATA_IID2, data2); // datastore matches, passed ok
tx.merge(LogicalDatastoreType.OPERATIONAL, DATA_IID, data); // datastore mismatches, exception thrown 

while this approach solves 'single datastore per transaction' while API remain as is,
in fact the update will bring no benefits: no optimization (same backing transactions require 
instantiation anyway), no inner logic modified, just introducing annoying extra restriction 
causing inobvious backward incompatibility (for those addressing different datastores in same 
transaction new exception will be introduced).

#2

imo more reasonable approach would be introducing new api using single store transactions,
with gradual migration (deprecate old api first, remove deprecated api and classes later).
so the transaction will be initially tied to single datastore, so it became unnecessary 
as an operation argument, the code will look like below

// new transaction interfaces, new databroker methods to build single datastore transactions
final DatastoreWriteTransaction tx = dataBroker.newWriteOnlyTransaction(LogicalDatastoreType.CONFIGURATION);
tx.merge(DATA_IID, data); // datastore is known already

while this approach seems more future-proof, it also requires more effort and a new set of
transaction related interfaces to be introduced

#3
another compromise (with keeping existing API) could be limiting number of  backed transactions if
is to add databroker method to build composite transaction with single backing transaction 

// old transaction interfaces, new databroker methods 
final WriteTransaction tx = dataBroker.newWriteOnlyTransaction(LogicalDatastoreType.CONFIGURATION);
tx.merge(LogicalDatastoreType.CONFIGURATION, DATA_IID, data); // ok
tx.merge(LogicalDatastoreType.OPERATIONAL, DATA_IID, data); // IllegalStateException, current behavior

the approach need to be clarified

 

Comment by Robert Varga [ 13/Oct/22 ]

Approach 1, definitely, as we need to catch violations first and allow implementations to rely on the fact this is not allowed.

We have datastore-bound versions of the WriteTransaction et al. prepared, but rolling those out is going to be a point.

Generated at Wed Feb 07 20:10:56 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.