ydb-platform / yoj-project

YDB ORM for Java (YOJ) is a lightweight ORM for immutable entities. It has native support for YDB and is battle-tested.
Apache License 2.0
13 stars 12 forks source link

better session handling #4

Closed lavrukov closed 10 months ago

lavrukov commented 11 months ago

If we received a TLI or BadSession inside a transaction, this means that the transaction was invalidated on the YDB side and sending commit() or rollback() is incorrect, since in this case they always generate a BadSession.

In order to write tests for this behavior that will work both on the real YDB and on the mock, we have to add a check for read locks inside the InMemoryRepository.

nvamelichev commented 11 months ago

I will look at the in-memory changes more thoroughly on Monday, January 8. New locking logic seems a bit strange, e.g. you can get a writable tx shard that (seemingly) never checks for TLI

lavrukov commented 11 months ago

I will look at the in-memory changes more thoroughly on Monday, January 8. New locking logic seems a bit strange, e.g. you can get a writable tx shard that (seemingly) never checks for TLI

Correct. I decided not to add lock checking for write operations because I could not reproduce the TLI for this case over a docker container. Looks like write operations can't return TLI right now. I showed it in test writeDontProduceTLI()

nvamelichev commented 11 months ago

Looks like write operations can't return TLI right now. I showed it in test writeDontProduceTLI()

Are we sure that this is the case always, for recent versions of YDB? Maybe it's better to be cautious and raise TLI here even if YDB currently does not.

lavrukov commented 10 months ago

Are we sure that this is the case always, for recent versions of YDB? Maybe it's better to be cautious and raise TLI here even if YDB currently does not.

We have no any guarantees about future, but now we can't reproduce it, it works without it and legacy in-memory don't have consistency checks on write operations too. Given all this, I don't think we have to enhance checks here.

nvamelichev commented 10 months ago

legacy in-memory don't have consistency checks on write operations too

This is a good argument :-)

I've looked at the code once again. Now I see that throwing TLI from writes will be an optimization, anyway: commit() will throw TLI if the data you wrote to in transaction has been changed by another transaction.