[foundationdb] add a new client for FoundationDB #1162
Conversation
|
In order to test the binding, you need to download the official java client from the links below and then install them manually to your local maven repo until the maven repo will be ready. https://www.foundationdb.org/downloads/5.1.7/bindings/java/fdb-java-5.1.7.jar
|
|
is there an issue filed with the foundationDB folks already about publishing to maven central or otherwise adding a repository? |
|
@busbey |
|
thanks for the pointer! we'll block on that issue closing out then. |
|
foundation db have published client jars to maven central now. FYI, @dbjoa. I'll see if restarting the travis build is enough. |
|
@busbey I've updated the PR. |
foundationdb/pom.xml
Outdated
| <name>FoundationDB Binding</name> | ||
| <packaging>jar</packaging> | ||
| <repositories> | ||
| <repository> |
There was a problem hiding this comment.
I don't think we need the snapshot repository. can we drop it?
There was a problem hiding this comment.
Thank you for the comment. I've dropped the snapshot repository from the PR.
| /** | ||
| * The YCSB binding for <a href="https://www.foundationdb.org">FoundationDB</a>. | ||
| */ | ||
| package com.yahoo.ycsb.db; |
There was a problem hiding this comment.
please put this into a dedicated package e.g. com.yahoo.ycsb.db.foundationdb
There was a problem hiding this comment.
Thank you for your suggestion. I've made the dedicated package.
| logger.info("Cluster File: {}\n", clusterFile); | ||
|
|
||
| try { | ||
| fdb = FDB.selectAPIVersion(Integer.parseInt(apiVersion.trim())); |
There was a problem hiding this comment.
is this the bit that needs to be protected by the synchronized block? is the db object itself threadsafe?
There was a problem hiding this comment.
Thank you for the comment. The synchronized block is not needed. I've removed it from the PR.
| } | ||
| if (curInitCount == 0) { | ||
| try { | ||
| db.close(); |
There was a problem hiding this comment.
over in init() we did fdb.open once for each client thread, but here we're only doing a single close. is that correct?
There was a problem hiding this comment.
Thank you for your detailed comment. db.close() should be called for each client thread. I've updated the PR.
|
@busbey I've changed the PR. |
foundationdb/pom.xml
Outdated
| <packaging>jar</packaging> | ||
| <properties> | ||
| <!-- Tests do not run on jdk9 --> | ||
| <skipJDK9Tests>true</skipJDK9Tests> |
There was a problem hiding this comment.
Sorry for the churn, but a PR just landed that adds JDK10 and JDK11 test runs. Could you add the additional skip properties for those?
There was a problem hiding this comment.
@busbey , No problem. I've added the skip properties for JDK10 and JDK11 to the updated PR.
|
Thanks for getting this done! |
Here is a new client for FoundationDB, which is based on the API version 5.1.7.