[orientdb] OrientDB was updated to 2.2.10 and bugs fixed #848

andrii0lomakin · 2016-09-26T14:14:21Z

Several mistakes in OrienDBClient (mostly related to initialization in multithreaded loads) were fixed.

…DBClient (mostly releated to initialization in multithreaded loads) were fixed.

risdenk · 2016-09-26T14:16:17Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

@@ -1,12 +1,12 @@
 /**
 * Copyright (c) 2012 - 2016 YCSB contributors. All rights reserved.
- *
+ * <p>


Header was changed?

Paragraphs were added automatically I removed it.

Looks like the
got put back by mistake. These should be removed.

risdenk · 2016-09-26T14:16:34Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

+ * orientdb.user=admin <br>
+ * orientdb.password=admin <br>
+ *
+ * @author Luca Garulli


No author tags

risdenk · 2016-09-26T14:17:00Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

+ * <p>
+ * Properties to set:
+ * <p>
+ * orientdb.url=local:C:/temp/databases or remote:localhost:2424 <br>


These properties are already in the readme?

The properties to set section should be removed since this is documented in the README. No need to document it twice.

risdenk · 2016-09-26T14:18:44Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java


-  private static final String URL_PROPERTY = "orientdb.url";


Looks like all the initialization code was changed? Including the property and default variables?

No, property and default values are the same.

Sorry my comment wasn't clear. The format of the entire class changed. The changes removed all the *_PROPERTY* variables and instead inlined all the values. This makes it hard to review. It is typically better to stick with the existing code format if possible.

@risdenk I added them back

risdenk · 2016-09-27T22:30:39Z

@kruthar Not sure you have time to review this, but I think you were the last to look at the OrientDB code.

…ote storage was added.

andrii0lomakin · 2016-09-28T13:49:12Z

Hi @risdenk , I have updated pull request according to your comments.

risdenk · 2016-09-28T15:45:27Z

orientdb/README.md

@@ -60,12 +60,6 @@ WARNING: Creating a new database will be done safely with multiple threads on a
 * ```orientdb.newdb``` - Overwrite the database if it already exists.
    * Only effects the ```load``` phase.
    * Default: ```false```
-* ```orientdb.intent``` - Declare an Intent to the database.


Assuming the orientdb.intent was removed in OrientDB 2.2?

@risdenk it is still present but does not have any effect and even sometimes has negative value.

Thanks for the explanation.

risdenk · 2016-09-28T17:04:58Z

@Laa Saw your comments about addressing some items but not sure the changes were pushed to this branch? ie: still see the author tags

andrii0lomakin · 2016-09-28T17:12:10Z

@risdenk what do you mean , authors tags should be present or absent ?

risdenk · 2016-09-28T17:13:59Z

There should be no author tags. Attribution is based on commit username.

andrii0lomakin · 2016-10-07T08:02:22Z

@risdenk done, tags are removed

risdenk

Some minor changes remain. Mostly around fixing headers, JavaDoc, and some minor test updates.

risdenk · 2016-10-11T13:46:46Z

orientdb/src/test/java/com/yahoo/ycsb/db/OrientDBClientTest.java

@@ -101,12 +102,20 @@ public void insertTest() {
    String insertKey = "user0";
    Map<String, ByteIterator> insertMap = insertRow(insertKey);

-    ODocument result = orientDBDictionary.get(insertKey);
+    OPartitionedDatabasePool pool = orientDBClient.getDatabasePool();
+    ODatabaseDocumentTx db = pool.acquire();


This can be a try w/ resources. Like other pool.acquire() calls.

risdenk · 2016-10-11T13:47:00Z

orientdb/src/test/java/com/yahoo/ycsb/db/OrientDBClientTest.java

-      for (int i = 0; i < NUM_FIELDS; i++) {
-        doc.field(FIELD_PREFIX + i, preupdateString);
+    OPartitionedDatabasePool pool = orientDBClient.getDatabasePool();
+    ODatabaseDocumentTx db = pool.acquire();


This can be a try w/ resources. Like other pool.acquire() calls.

risdenk · 2016-10-11T13:48:15Z

orientdb/src/test/java/com/yahoo/ycsb/db/OrientDBClientTest.java

@@ -227,10 +259,11 @@ public void scanTest() {
    int testIndex = startIndex;

    // Check each vector row to make sure we have the correct fields
-    for (HashMap<String, ByteIterator> result: resultVector) {
+    for (HashMap<String, ByteIterator> result : resultVector) {


The comment a few lines above this about index starting at 1 can be removed. Change line 243 to startIndex = 0.

risdenk · 2016-10-11T13:48:24Z

orientdb/src/test/java/com/yahoo/ycsb/db/OrientDBClientTest.java

-    assertNull("Assert user1 does not exist", orientDBDictionary.get(user1));
-    assertNotNull("Assert user2 still exists", orientDBDictionary.get(user2));
+    OPartitionedDatabasePool pool = orientDBClient.getDatabasePool();
+    ODatabaseDocumentTx db = pool.acquire();


This can be a try w/ resources. Like other pool.acquire() calls.

risdenk · 2016-10-11T13:48:43Z

orientdb/src/test/java/com/yahoo/ycsb/db/OrientDBClientTest.java

@@ -1,12 +1,12 @@
 /**
 * Copyright (c) 2015 - 2016 YCSB contributors. All rights reserved.
- *
+ * <p>


Remove added
from header.

risdenk · 2016-10-11T13:49:25Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

  }

+  /**


This JavaDoc is unnecessary since it is documented on the base class.

risdenk · 2016-10-11T13:49:27Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

@@ -217,20 +230,43 @@ public Status insert(String table, String key, HashMap<String, ByteIterator> val
    return Status.ERROR;
  }

+  /**


This JavaDoc is unnecessary since it is documented on the base class.

risdenk · 2016-10-11T13:49:31Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

  }

+  /**


This JavaDoc is unnecessary since it is documented on the base class.

risdenk · 2016-10-11T13:50:04Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

+ * <p>
+ * Properties to set:
+ * <p>
+ * orientdb.url=local:C:/temp/databases or remote:localhost:2424 <br>


The properties to set section should be removed since this is documented in the README. No need to document it twice.

risdenk · 2016-10-11T13:50:21Z

orientdb/src/main/java/com/yahoo/ycsb/db/OrientDBClient.java

@@ -1,12 +1,12 @@
 /**
 * Copyright (c) 2012 - 2016 YCSB contributors. All rights reserved.
- *
+ * <p>


Looks like the
got put back by mistake. These should be removed.

andrii0lomakin · 2016-10-31T19:38:34Z

@risdenk sorry for delay seems all observations are fixed.

risdenk · 2016-10-31T19:54:00Z

@Laa changes look good. I'm pulling it down locally to check and waiting on CI tests to finish. Is it possible for you to squash the commits and start the single commit message with [orientdb]?

risdenk · 2016-10-31T20:27:07Z

@Laa changes and tests pass. It looks like I can squash and merge through Github. I'll do that in a few minutes. Thanks for your patience!

OrientDB version was updated till 2.2.10 , several mistakes in Orient…

fda350f

…DBClient (mostly releated to initialization in multithreaded loads) were fixed.

risdenk requested changes Sep 26, 2016

View reviewed changes

Pullrequest brianfrankcooper#848. Readme was updates, support for rem…

3974555

…ote storage was added.

risdenk reviewed Sep 28, 2016

View reviewed changes

risdenk added bug db-update labels Sep 28, 2016

Pullrequest brianfrankcooper#848. Author tags are removed

3621d3c

risdenk requested changes Oct 11, 2016

View reviewed changes

Pullrequest brianfrankcooper#848 JavaDoc fixex

88b7ebc

andrii0lomakin added 2 commits October 31, 2016 21:45

Pullrequest brianfrankcooper#848 JavaDoc fixes

828fe3b

Merge branch 'master' of github.com:laa/YCSB

8ecfb7a

risdenk approved these changes Oct 31, 2016

View reviewed changes

risdenk changed the title ~~OrientDB version was updated till 2.2.10, mistakes are fixed~~ [orientdb] OrientDB was updated to 2.2.10 and bugs fixed Oct 31, 2016

risdenk merged commit 70f4362 into brianfrankcooper:master Oct 31, 2016

risdenk mentioned this pull request Nov 15, 2016

Release version 0.12.0 #873

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[orientdb] OrientDB was updated to 2.2.10 and bugs fixed #848

[orientdb] OrientDB was updated to 2.2.10 and bugs fixed #848

andrii0lomakin commented Sep 26, 2016

risdenk Sep 26, 2016

andrii0lomakin Sep 28, 2016

risdenk Oct 11, 2016

risdenk Sep 26, 2016

andrii0lomakin Sep 28, 2016

risdenk Sep 26, 2016

risdenk Oct 11, 2016

risdenk Sep 26, 2016

andrii0lomakin Sep 28, 2016

risdenk Sep 28, 2016

andrii0lomakin Sep 28, 2016

risdenk commented Sep 27, 2016

andrii0lomakin commented Sep 28, 2016

risdenk Sep 28, 2016

andrii0lomakin Sep 28, 2016

risdenk Sep 28, 2016

risdenk commented Sep 28, 2016

andrii0lomakin commented Sep 28, 2016

risdenk commented Sep 28, 2016

andrii0lomakin commented Oct 7, 2016

risdenk left a comment

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

risdenk Oct 11, 2016

andrii0lomakin commented Oct 31, 2016

risdenk commented Oct 31, 2016

risdenk commented Oct 31, 2016

[orientdb] OrientDB was updated to 2.2.10 and bugs fixed #848

[orientdb] OrientDB was updated to 2.2.10 and bugs fixed #848

Conversation

andrii0lomakin commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

risdenk commented Sep 27, 2016

andrii0lomakin commented Sep 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

risdenk commented Sep 28, 2016

andrii0lomakin commented Sep 28, 2016

risdenk commented Sep 28, 2016

andrii0lomakin commented Oct 7, 2016

risdenk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrii0lomakin commented Oct 31, 2016

risdenk commented Oct 31, 2016

risdenk commented Oct 31, 2016