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

Positive Only does not work with concurrent calls #114

Open
sudhirj opened this issue Apr 19, 2016 · 3 comments
Open

Positive Only does not work with concurrent calls #114

sudhirj opened this issue Apr 19, 2016 · 3 comments

Comments

@sudhirj
Copy link

sudhirj commented Apr 19, 2016

I have a ledger running with an account type set to positive_only true. See the Wallet account type in the following:

https://github.com/sudhirj/notecase/blob/master/config/initializers/double_entry.rb

This works fine normally, but on a very small minority of cases where transactions come in at almost the same time, I've seen account balances go negative. Is there anything that can be done to avoid this? Both accounts are being locked as follows:

https://github.com/sudhirj/notecase/blob/master/app/models/transaction.rb

Any ideas on how we can improve the integrity of positive_only accounts?

@orien
Copy link
Member

orien commented Nov 24, 2016

This is indeed worrying. The account locking should prevent this from happening. The jack hammer script attempts to prove this is the case. What are the specifics of your environment @sudhirj? I wonder if we can reproduce this via the jack hammer script.

@KelseyDH
Copy link
Contributor

KelseyDH commented Feb 21, 2018

Looking at your method, two possible causes include the fact that it's a class method, which if you're running this job on a multi-threaded worker like Sidekiq could possibly cause race conditions for any instance variables instantiated.

The other possible cause could be here:

        transaction = self.where(ref: ref).first
        return unless transaction.nil? # a transaction with this ref has already been created. Exit.

Where I suspect a race condition could be happening here due to delays in either an ActiveRecord query cache, or that two objects could get loaded here and pass this nil check while one is in the process of creating a transaction.

If I'm not mistaken, locks and transactions only prevent consistency errors at the database level. In this case if two jobs were running, my sense is that both jobs would run, but with the effect of the lock being that one of them would just need to wait their turn first, before actually getting processed by double_entry. So that means consistency in terms of no weird double spending at the database level, but not consistency in terms of preventing duplicate transactions?

(I am speculating here, but hope it helps!)

@sudhirj
Copy link
Author

sudhirj commented Feb 21, 2018

While this could certainly explain any possible race conditions in the application code and database, it wouldn't explain the negative balance in the DoubleEntry managed tables - the handover of a transaction to double entry is in this line

        DoubleEntry.transfer Money.new(amount),
                from: from_account,
                to: to_account,
                detail: transaction,
                code: self.name.downcase.to_sym # DoubleEntry will make sure only the registered subclasses are used

which is a clean request that should never succeed if the guarantees being offered by the gem hold true. i.e. the double entry code should throw an error irrespective of the quality or raciness of the inputs.

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

3 participants