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

[orientdb] put db conn stuff in sync method #609

Merged
merged 1 commit into from
Feb 10, 2016

Conversation

kruthar
Copy link
Collaborator

@kruthar kruthar commented Feb 2, 2016

All db connection and creation logic is the same, it is just pulled into
a synchronized method so that multiple threads run through the
connection process one at a time.

Fixes #571

@Override
public void init() throws DBException {
Properties props = getProperties();
private static final Logger LOG = LoggerFactory.getLogger(OrientDBClient.CLASS);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a lower case ".class"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch. OrientDBClient.CLASS="usertable".

@risdenk
Copy link
Collaborator

risdenk commented Feb 7, 2016

Would there be issues here if multiple YCSB clients are run at once? This will only help fix multiple threads in one client correct?

Does the initdb need to create the db? Is it possible to make it necessary to have the db already exist as a prerequisite? Like the other bindings ie: Solr and Cassandra

@busbey
Copy link
Collaborator

busbey commented Feb 7, 2016

We could add a note to the README for the folks who use multiple Client invocations, letting them know how to create it in advance?

@kruthar
Copy link
Collaborator Author

kruthar commented Feb 7, 2016

@risdenk

Would there be issues here if multiple YCSB clients are run at once? This will only help fix multiple threads in one client correct?

Correct.

Does the initdb need to create the db? Is it possible to make it necessary to have the db already exist as a prerequisite? Like the other bindings ie: Solr and Cassandra

So, the OrientDB client is the only one that I am aware of that actually creates a database for you. All others require it to be present. I kept this functionality because it was there from the initial creation of the client. I'm ok with removing it, or just writing a note about that specific scenario.

@busbey
Copy link
Collaborator

busbey commented Feb 7, 2016

Either a note or removal sounds fine by me.

Could also move the creation into a utility class that the README references. I think I've seen a couple of those.

All db connection and creation logic is the same, it is just pulled into
a synchronized method so that multiple threads run through the
connection process one at a time.

Fixes brianfrankcooper#571
@kruthar
Copy link
Collaborator Author

kruthar commented Feb 10, 2016

I decided to keep the db creation functionality in place since 1) it works and 2) mongodb does "create" a database if it doesn't already exist, although in a much more passive way.

I added a note about it to the README

@busbey
Copy link
Collaborator

busbey commented Feb 10, 2016

+1

kruthar added a commit that referenced this pull request Feb 10, 2016
[orientdb] put db conn stuff in sync method
@kruthar kruthar merged commit 9fc8b68 into brianfrankcooper:master Feb 10, 2016
@kruthar kruthar deleted the orientdb-create branch February 10, 2016 03:33
@risdenk risdenk mentioned this pull request Feb 15, 2016
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[orientdb] put db conn stuff in sync method
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[orientdb] put db conn stuff in sync method
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.

3 participants