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

YCSB+T: Added transactions to DB and DBWrapper and validate() to Workloa... #169

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

Conversation

akon-dey
Copy link

...d.

YCSB+T: Added an example ClosedEconomyWorkload with a simple implementation of validate and a Simple Anomaly Score.

…load.

YCSB+T: Added an example ClosedEconomyWorkload with a simple implementation of validate and a Simple Anomaly Score.
@akon-dey akon-dey closed this Apr 14, 2014
@akon-dey akon-dey reopened this Apr 14, 2014
Benjamin and others added 8 commits December 12, 2014 10:06
Fixes MongoExceptions that were being raised previously
This branch fixes two bugs in MongoDbClient:

ClassCastExceptions raised due to the incorrect handling of field values returned by database queries in read()
MongoExceptions raised due to attempts to update the _id field value in update()
The return value of DB.update() was previously unchecked in
ClosedEconomyWorkload.doTransactionReadModifyWrite(), leading DB.abort() not
being called when a transaction failed half-way
Check DB.update() return value in ClosedEconomyWorkload.doTransactionReadModifyWrite()
Fix variable swap bug so that ClosedEconomyWorkload only moves money from higher-numbered accounts/keys to lower-numbered accounts/keys
@busbey
Copy link
Collaborator

busbey commented Aug 6, 2015

bump on this issue. would like to see this resolved for the Sep release.

@busbey
Copy link
Collaborator

busbey commented Aug 6, 2015

since this changes the core binding definition it'll probably take some work to get right. /cc @allanbank since the patch as is touches the Mongo code, though I've no reason to believe it will need substantially different handling than other datastores.

open quesiton: if we have commit / abort, what do we do with datastores that have no notion of transactions?

@akon-dey
Copy link
Author

akon-dey commented Aug 6, 2015

The default start(), commit() and abort() are defined as empty methods in DB.java. This is similar to the cleanup() and init() default implementations.

@busbey
Copy link
Collaborator

busbey commented Aug 6, 2015

ah. and the default consistency check just says yes blindly. I see. could merge that with the new verify stuff in core workload.

thanks for the pointer!

Conflicts:
	core/src/main/java/com/yahoo/ycsb/Client.java
	core/src/main/java/com/yahoo/ycsb/workloads/CoreWorkload.java
	mongodb/src/main/java/com/yahoo/ycsb/db/MongoDbClient.java
@akon-dey
Copy link
Author

I have merged the changes but the verification code is not fully integrated yet. I'd be happy to help with it.

}
catch (InterruptedException e)
{
// do nothing.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably wrong. we should at a minimum set the interrupted status of the thread.

@busbey
Copy link
Collaborator

busbey commented Aug 10, 2015

please don't include additional format fixes, it makes it harder to review the substance of the change.

//and the sleep() doesn't make sense for granularities < 1 ms anyway
if ( (_targetOpsPerMs>0) && (_targetOpsPerMs<=1.0) )
{
sleep(Utils.random().nextInt((int)(1.0/_targetOpsTickNs)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks unrelated to the change mentioned in the PR description, could you leave it out?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the format changes. I could merge it again if that helps.

@uakfdotb
Copy link

It seems like the update operations reset records to the initial value (totalcash / recordcount), if there has been a readmodifywrite performed on the record then it seems like this breaks the closed economy invariant.

@akon-dey
Copy link
Author

Please review the recent changes and let me know if you want me to address anything.

@0xl3x1
Copy link
Contributor

0xl3x1 commented Feb 11, 2016

@akon-dey I wrote a very similar start(), commit(), abort() implementation for JdbcDBClient last year, except I added the ability to set the transaction isolation level (see https://github.com/benjaminbrent/YCSB/blob/e2aa4f4be142afc2acfd98e5c3e22bd659da8d01/jdbc/src/main/java/com/yahoo/ycsb/db/JdbcDBClient.java#L255).

It would be nice to have this in your implementation too. I could submit a pull request if you think this is worthwhile?

@akon-dey
Copy link
Author

This would be useful. We should be able to set the isolation level using the workload configuration properties file. That will make it truly usable.

@akon-dey
Copy link
Author

Please submit a pull request with the ability to set the workload properties.

@morcelicaio
Copy link

Hello @akon-dey .

Have you noticed the behavior of ycsb+t when running the load phase (in the ClosedEconomyWorkload layer) using the parameter w = 0 in the mongo db cluster?

It simply gives a NumberFormatException error on the last record (in the ClosedEconomyWorkload layer).

I'm running on Mongodb Atlas and I noticed this error doing tests.

Do you have any idea about this behavior?

@akon-dey
Copy link
Author

@morcelicaio I have not experienced it. If you can send me a stack or better, a pull request, I would be happy to incorporate it.

@morcelicaio
Copy link

@akon-dey , I did some more tests on another computer and didn't get the same error. Maybe it could have been some point on the other machine. If I get the same error again, I'll let you know. Thanks!

@morcelicaio
Copy link

Hi @akon-dey and @busbey , I would like to ask another question.

Wasn't YCSB+T merged with the original YCSB?

As I understand it, in the latest available version of YCSB there is no transactional layer you added. I would like to know the status of this.

Thanks!

@akon-dey
Copy link
Author

@morcelicaio I don't believe it was merged. There were some changes to the PR that had questions and hence not merged. One was code format related and the other was regarding completeness of the ClosedEconomyWorkload. The first was trivial and the second was not intended to be addressed by the code. It would be best to recreate the PR to address the first issue and merge if it looks good.

@busbey
Copy link
Collaborator

busbey commented Apr 19, 2022

it has not yet been merged. There's also merge conflicts, so it will need to go through a rebase before review.

@morcelicaio
Copy link

Hello guys, @busbey, @akon-dey .

Regarding YCSB+T, he talks about evaluating the transactional part of NoSQL databases.

I'm still testing with MongoDB.
I noticed that in the YCSB+T implementation we don't have the code implemented that creates the transaction as it is in the MongoDB documentation Transaction API (https://www.mongodb.com/docs/manual/core/transactions/).

Do I need to implement this code to use YCSB+T?
Or do the start() abort() and commit() methods somehow perform these "transactions" implicitly?

Thanks

@busbey
Copy link
Collaborator

busbey commented Nov 8, 2022

I would assume that if you don't see the mongo transaction API being called then probably the existing code is not making use of it.

@morcelicaio
Copy link

Just that, @busbey . So how does YCSB+T assess transactional overhead and have a layer to assess transactional consistency when the transaction code implementation is not in the YCSB+T code?

It's a question that maybe other users have as well.

@busbey
Copy link
Collaborator

busbey commented Nov 9, 2022

as mentioned previously, this proposed change hasn't been merged yet. It also started in 2014, so it's possible it predates the mongo transactional api.

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

Successfully merging this pull request may close these issues.

7 participants