impl(spanner): failing demonstration test #15391
Conversation
|
|
||
| // Transaction is created before the usual Connection and Client creation. | ||
| // This is the crux of reproducing this issue. | ||
| auto txn = spanner::Transaction(spanner::Transaction::ReadWriteOptions{}); |
There was a problem hiding this comment.
If you want the ExcludeTransactionFromChangeStreamsOption to have an effect, you have to instantiate it before you create the transaction.
That is, in general you need to build the OptionsSpan before you call into the library.
There was a problem hiding this comment.
Agreed. However the current API doesn't enforce that. If the user creates a Transaction first, then creates the Connection and Client, they may not get the behavior they expect and there is no diagnostic indicating such.
There was a problem hiding this comment.
These operations can come in any order you like, modulo their data dependencies (and the test expectations that would have to be tweaked here due to the lifetime differences).
The issue is not the relative ordering of the Transaction and Connection/Client, but rather the ordering of the Transaction and the OptionsSpan. As mentioned above, "If you want the ExcludeTransactionFromChangeStreamsOption to have an effect, you have to instantiate it before you create the transaction." Write before read.
There is no way to enforce/diagnose that because the Option setting is, after all, optional.
scotthart
left a comment
scotthart
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @devbww)
|
|
||
| // Transaction is created before the usual Connection and Client creation. | ||
| // This is the crux of reproducing this issue. | ||
| auto txn = spanner::Transaction(spanner::Transaction::ReadWriteOptions{}); |
There was a problem hiding this comment.
Agreed. However the current API doesn't enforce that. If the user creates a Transaction first, then creates the Connection and Client, they may not get the behavior they expect and there is no diagnostic indicating such.
devbww
left a comment
devbww
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @scotthart)
|
|
||
| // Transaction is created before the usual Connection and Client creation. | ||
| // This is the crux of reproducing this issue. | ||
| auto txn = spanner::Transaction(spanner::Transaction::ReadWriteOptions{}); |
There was a problem hiding this comment.
These operations can come in any order you like, modulo their data dependencies (and the test expectations that would have to be tweaked here due to the lifetime differences).
The issue is not the relative ordering of the Transaction and Connection/Client, but rather the ordering of the Transaction and the OptionsSpan. As mentioned above, "If you want the ExcludeTransactionFromChangeStreamsOption to have an effect, you have to instantiate it before you create the transaction." Write before read.
There is no way to enforce/diagnose that because the Option setting is, after all, optional.
2 participants
This change is