Skip to content
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

Transaction in Replicate not block global #147

Closed
thachtan opened this issue Feb 26, 2022 · 6 comments
Closed

Transaction in Replicate not block global #147

thachtan opened this issue Feb 26, 2022 · 6 comments

Comments

@thachtan
Copy link

I have some trouble with replicate adapter when using transaction. a process in another node still update/put cache with the same key.
I have mini repo describe this issue.

Context in example:

  • start app in two nodes (:node1, :node2)
  • account balance in starting is 0.
  • get cache function will create transaction and add 100, 200 in balance.
  • run get cache function in both nodes in same time
  • result still 300 not 600 as expect.

Screen Shot 2022-02-26 at 11 06 57

Screen Shot 2022-02-26 at 11 07 19

I think this cause by Nebulex.Adapter.Transaction only block the current pid of cache app

@cabol
Copy link
Owner

cabol commented Mar 4, 2022

Hey 👋 ! I will try to take a look at it very soon, BTW, thanks for the detailed information about the issue, that helps a lot :)

@cabol
Copy link
Owner

cabol commented Mar 20, 2022

Hi @thachtan 👋 ! First of all, apologies for the lateness, I've been pretty busy so I hadn't been able to check this out.

Now, I ran the example you have, and did work for me as expected, but I had to fix some issues you had in your code, let me list them here:

  1. You are using the wrong app name to load the config. In the DemoNebulexLockKey.Application module when you set up the cluster you are using :demo_fifo_nebulex_ecto instead of :demo_nebulex_lock_key. I think this is the main mistake and the one causing the issue, because of this the cache cluster is not set up properly, which means, the nodes won't see each other. Try to find it and let me know if only with that fix it works.

  2. In your config.exs file, the correct configuration for the replicated cache should be like so:

config :demo_nebulex_lock_key, ReplicatedCache,
  stats: true,
  primary: [
    gc_interval: :timer.hours(12),
    max_size: 1_000_000,
    allocated_memory: 2_000_000_000,
    gc_cleanup_min_timeout: :timer.seconds(10),
    gc_cleanup_max_timeout: :timer.minutes(10),
    backend: :shards,
    partitions: System.schedulers_online() * 2 #=> Beaware this is resolved in compile-time
  ]

The issue here is the primary storage config must be within the option :primary. Also, you don't need :nodes and :name (by default the name of the cache is the same module name, which is ReplicatedCache).

  1. In the DemoNebulexLockKey module, specifically HERE, you don't need to pass the nodes:, those are automatically resolved by the adapter, the only thing to ensure is the proper cluster setup.

Summing up, as I mentioned before, after fixing your code (especially/mainly fix no. 1), I got:

Node 1:

iex(node1@127.0.0.1)1> DemoNebulexLockKey.get_cache(1)
true
account after deposit 100: %{balance: 400, id: 1, name: "account 1"}
account after add 200: %{balance: 600, id: 1, name: "account 1"}
%{balance: 600, id: 1, name: "account 1"}

Node 2:

iex(node2@127.0.0.1)1> DemoNebulexLockKey.get_cache(1)
true
account after deposit 100: %{balance: 200, id: 1, name: "account 1"}
account after add 200: %{balance: 600, id: 1, name: "account 1"}
%{balance: 600, id: 1, name: "account 1"}

Try to fix it as I mentioned, and let me know how it goes, stay tuned!

@thachtan
Copy link
Author

Oh, i apologize for my bad config and take time of you. Sorry about i have no time to reply soon.

But I remembered true error. I have updated new version with demo.

I will explain now.

First I have updated config as you instruction. Next I run get and update account balance with transaction in the same time. But it will have different at start time run. Node 1 will sleep 5s, node 2 will sleep 10s.

And it run as diagram below.
nebulex drawio

And result of terminal of two nodes.
Screen Shot 2022-03-27 at 12 29 40
Screen Shot 2022-03-27 at 12 29 57

And my issues is Why it not block transaction in node 2 and why Node 2 still update account balance when node 1 is run first.

As my think before:
"I think this cause by Nebulex.Adapter.Transaction only block the current pid of cache app"

Thank you for check this again.

@cabol
Copy link
Owner

cabol commented Mar 28, 2022

Hey 👋 !

You were right, there was an issue with the transaction logic, but I just pushed a fix to the master branch. If I run your demo with the fix:

iex(node1@127.0.0.1)1> DemoNebulexLockKey.get_cache(1, 5000)
true
account before deposit 100: %{balance: 0, id: 1, name: "account 1"}
account after deposit 100: %{balance: 100, id: 1, name: "account 1"}
account after add 200: %{balance: 300, id: 1, name: "account 1"}
%{balance: 300, id: 1, name: "account 1"}

Just to point out the node2 now states blocked until the transaction in node1 finishes:

iex(node2@127.0.0.1)1> DemoNebulexLockKey.get_cache(1, 10000)

Then the result is:

iex(node2@127.0.0.1)1> DemoNebulexLockKey.get_cache(1, 10000)
true
account before deposit 100: %{balance: 300, id: 1, name: "account 1"}
account after deposit 100: %{balance: 400, id: 1, name: "account 1"}
account after add 200: %{balance: 600, id: 1, name: "account 1"}
%{balance: 600, id: 1, name: "account 1"}

Please try to test again your demo with the fix and let me know if it works for you, if so, I will publish a new patch release ASAP.

Thanks!

@thachtan
Copy link
Author

Oh it works as my expected. Thank you for take time to help this issues.

@cabol
Copy link
Owner

cabol commented Mar 29, 2022

Excellent! And thanks to you too, it was an important issue, and you provided a lot of details so that made the work much easier, thanks!!

BTW, a new release 2.3.2 was published already with the fix!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants