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

Adding Thread Safe ability on HaeinsaTransaction for Put / Delete / Get with different hTable #48

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ehudl
Copy link

@ehudl ehudl commented Aug 18, 2015

Currently we cannot run the following scenario:

  1. Create manager.// one thread
  2. Begin transaction. // one thread
  3. Run:
    Put / Delete / Get. // with the same transaction with different threads.
  4. On success of all Threads -> commit // one thread.

The fix I am suggesting will not change the current API at all, it will only add the ability to use some locks on mutation collections (using java.concurrent library).

I added a test to illustrate the scenario.

We would appreciate it if you would pull the attached code. Furthermore, any feedback would be welcome.


This change is Reviewable

@eincs
Copy link
Contributor

eincs commented Aug 25, 2015

How do you think about this scenario?

  1. MainThread: tx=BeginTransaction
  2. Thread1: value1=get(tx, key1)
  3. Thread1: newValue1=value1+1
  4. Thread2: value1=get(tx, key1)
  5. Thread2: newValue2=value1+1
  6. Thread1: put(tx, key1, newValue1)
  7. Thread2: put(tx, key1, newValue2)
  8. MainThread: On success of all Threads, tx.commit()

I think user would expect that value stored in key1 should be value1+2. But actual value might be value+1. In my opinion, to support multi-threaded transaction, this problem should be solved. How do you think?

I'm now looking for how other transaction system handle this problem.

@ehudl
Copy link
Author

ehudl commented Aug 27, 2015

Yes this is obviously a problem, but as I see it this is a wrong usage of the library.

I made this ability to enable running a "command" where each command only works on it's own rows, and each command can run in a different thread.

So maybe the definition of "thread safe" is not the best, but the point is working on multiple rows. As long as the rows are different you should be thread safe.

The mutli-threaded ability we added was to optimize the bounding between I/O and CPU. We are optimizing the concurrency on the I/O.
When running the example you describe, you actually do the opposite since it is preferable for you to get the same row with the same thread. The other direction is not efficient.

So this scenario is not possible in our system , but as I see it, this scenario is also not relevant.

Maybe we should add better documentation or change the name of the Flag.

Anyhow, we appreciate your advice.

@ehudl
Copy link
Author

ehudl commented Sep 7, 2015

I added some code to protect this case.
I am aware that by using this code I prevent the option of mutating the same row more than once, even with the same thread.

What do you think about this ?

@0mok 0mok self-requested a review January 2, 2017 04:51
@0mok 0mok self-assigned this Jan 2, 2017
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

Successfully merging this pull request may close these issues.

4 participants