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

[arangodb3] added ArangoDB3 binding (ArangoDB 3.1, Java driver 4.1.7) #903

Merged
merged 8 commits into from
Feb 2, 2017
Merged

Conversation

mvollmary
Copy link
Contributor

Updated ArangoDB binding with the official Java driver in version 4.1.5 which supports ArangoDB 3.1 and above.

The driver supports ArangoDBs new binary protocol – VelocyStream – to transport VelocyPack (internal storage format of ArangoDB introduced with the 3.0 release) data between ArangoDB and client applications.

@risdenk
Copy link
Collaborator

risdenk commented Jan 19, 2017

@mpv1989 thanks for the PR. Does this mean that this will not work with ArangoDB 2.x now? Would there be a case for trying to compare AragoDB 2.x with 3.x?

@mvollmary
Copy link
Contributor Author

@risdenk Yes, this will not work with ArangoDB 2.x anymore. I don't think a comparison between ArangoDB 2.x and 3.x is relevant. Maybe a comparison between the old Java driver and the new, both with ArangoDB 3.1.x would be interesting because the new driver offers VelocyStream/VelocyPack which results in a better performance than the old one. On the other hand, the old driver will be probably supported not for long anymore.

@risdenk
Copy link
Collaborator

risdenk commented Jan 27, 2017

If that is the case then maybe it makes sense to add an arrangodb3 binding so the old one still exists for the time being. We can come up with a plan to remove the old binding at some point.

@mvollmary mvollmary changed the title [arangodb] Updated ArangoDB binding (ArangoDB 3.1, Java driver 4.1.5) [arangodb] added ArangoDB3 binding (ArangoDB 3.1, Java driver 4.1.7) Feb 2, 2017
@mvollmary
Copy link
Contributor Author

@risdenk good idea. I reverted my changes and added an arangodb3 binding

@risdenk risdenk changed the title [arangodb] added ArangoDB3 binding (ArangoDB 3.1, Java driver 4.1.7) [arangodb3] added ArangoDB3 binding (ArangoDB 3.1, Java driver 4.1.7) Feb 2, 2017
Copy link
Collaborator

@risdenk risdenk left a comment

Choose a reason for hiding this comment

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

@mpv1989 Thanks for the updated PR. I added some comments.

@@ -0,0 +1 @@
/bin/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary? Just curious why.

@@ -0,0 +1,93 @@
<!--
Copyright (c) 2012 - 2015 YCSB contributors. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since these are new files can they just be 2017? Should be the same for the other files too.


## Quick Start

This section describes how to run YCSB on ArangoDB.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add 3.x or something in here?


<configuration>

<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these messages go to STDERR instead? The convention is to have only the YCSB measurement output go to STDOUT. This makes it easier to split out when running tests.

* permissions and limitations under the License. See accompanying
* LICENSE file.
*/
package com.yahoo.ycsb.db;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the package be com.yahoo.ycsb.db.arrangodb? This affects a few other places as well.

This change is because we are slowly moving the bindings to their own packages.

* The database name to access.
*/
private static String databaseName = "ycsb";
private static String collectionName = "usertable";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are databaseName and collectionName not customizable via properties?

Also can these variables be private without static? Same with the variables dropDBBeforeRun, waitforSync, and transactionUpdate

@risdenk risdenk added this to the 0.13.0 milestone Feb 2, 2017
@risdenk risdenk self-assigned this Feb 2, 2017
@risdenk
Copy link
Collaborator

risdenk commented Feb 2, 2017

Thanks for the changes @mpv1989 I'll pull it down and test.

@risdenk risdenk merged commit 9336c2c into brianfrankcooper:master Feb 2, 2017
@manolama manolama mentioned this pull request Sep 22, 2017
@busbey busbey mentioned this pull request May 19, 2018
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.

2 participants