New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: GlobalIdPK cache is not clear when transaction rollback has been performed #467

Closed
YaraslauBarysenka opened this Issue Nov 21, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@YaraslauBarysenka

YaraslauBarysenka commented Nov 21, 2016

Expectations versus reality

Preconditions

  • We use Javers 2.7.1 version.
  • A database have already populated with data.
  • Transaction has been started.
  • Audit has happened.
  • The transaction is failed and the transaction rollback was performed.

Expected Result

  • The globalIdPkCache (in GlobalIdRepository class) should be cleared when a transaction rollback was performed.

Current Result

  • Necessary rows were rollbacked from jv_global_id table, but globalId is present in cache(GlobalIdRepository class). When we try to make audit with an entity from previoust transaction and javers commit operation get globalId from cashe, the globalId is not present in database. If that Javers tries to create new snapshot then we receive an exception(see exception here: exception.txt), because Javers inserts not existing primary key.

Runnable test case

The application to reproduce issue here: javers-bug-example.zip

Please, read an instruction about how to reproduce the issue using the application here: instrution.txt

P. S.

If you have questions, please let's me know.
Guys, thank you for your work. It is very helpful for us.
Thank you in advance.

@YaraslauBarysenka YaraslauBarysenka changed the title from GlobalIdPK cache is not clear when transaction rollback has been performed to Bug: GlobalIdPK cache is not clear when transaction rollback has been performed Nov 21, 2016

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 21, 2016

First, thanks for a high quality bug report. You are right, this cache is simply non-transactional. Did you looked at javers source ? Maybe you are eager to contribute a Pull Request with a fix?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 21, 2016

Making this cache transactional could be hard. Maybe good error handling is enough, cache clean + retry

@YaraslauBarysenka

This comment has been minimized.

YaraslauBarysenka commented Nov 22, 2016

Sorry, we don't have enough resources to make a pull request.
As workaround, could you please provide possibility set custom cashe to storage GlobalIdPK?

Provide method to set custom cache by interface when we build Javers object and make public method to get this cashe?

Like this:
interface Cache {
// .. methods
}

Cache customCache = new CustomCacheImpl();
JaversBuilder.javers().setCache(customCache).build();

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 23, 2016

I will provide a method to evict the cache, you will be calling this method after doing rollback, ok?

@edmallia

This comment has been minimized.

edmallia commented Nov 23, 2016

Would it be possible to expose configuration to configure the cache including disabling it ?

@YaraslauBarysenka

This comment has been minimized.

YaraslauBarysenka commented Nov 23, 2016

It is nice to make possibility to evict a cache, but would be great to make a cache configurable.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 24, 2016

ok, disable switch for the cache seems reasonable

bartoszwalacik added a commit that referenced this issue Nov 24, 2016

#467
disable cache switch, evict cache method
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 25, 2016

The easy part is done, now we should configure Spring Transaction Manager to call our new cache evict() method when a SQL transaction is rolled back.
I don't know how to do it. I've posted a question on stack: http://stackoverflow.com/questions/40812130/how-application-can-be-notified-when-spring-transaction-manager-rollbacks-a-sql
Maybe someone from you Guys can help?

bartoszwalacik added a commit that referenced this issue Nov 27, 2016

#467
evict cache on transaction rollback

bartoszwalacik added a commit that referenced this issue Nov 28, 2016

#467
test fixes

bartoszwalacik added a commit that referenced this issue Nov 29, 2016

#467
isSynchronizationActive

bartoszwalacik added a commit that referenced this issue Nov 29, 2016

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 29, 2016

fixed in 2.7.2. You don't need to change your configuration, just use the new version. Now, when Spring Transaction Manager rolls back a transaction, the cache is automatically evicted.

@YaraslauBarysenka

This comment has been minimized.

YaraslauBarysenka commented Nov 30, 2016

Thank you for your work, but the issue is still reproducible.
I have create new issue #472

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Dec 2, 2016

This issue is fixed in javers-spring integration module. See my comment in #472 for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment