You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by chunhui-shi <gi...@git.apache.org> on 2017/02/04 00:10:31 UTC

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

GitHub user chunhui-shi opened a pull request:

    https://github.com/apache/drill/pull/741

    DRILL-5196: init MongoDB cluster when run a single test case directly\u2026

    \u2026 through command line or IDE.
    
    Other fixes include:
    Sync mongo-java-driver versions to newer 3.2.0
    update flapdoodle package to latest accordingly

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/chunhui-shi/drill DRILL-5196

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/741.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #741
    
----
commit 6068d7a21a0cf45ba9f5f3c76ad48dbd94c6e204
Author: chunhui-shi <cs...@maprtech.com>
Date:   2017-01-14T01:20:46Z

    DRILL-5196: init MongoDB cluster when run a single test case directly through command line or IDE.
    Other fixes include:
    Sync mongo-java-driver versions to newer 3.2.0
    update flapdoodle package to latest accordingly

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by chunhui-shi <gi...@git.apache.org>.
Github user chunhui-shi commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99453293
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -234,15 +243,25 @@ private static void createDbAndCollections(String dbName,
     
       @AfterClass
       public static void tearDownCluster() throws Exception {
    -    if (mongoClient != null) {
    -      mongoClient.dropDatabase(EMPLOYEE_DB);
    -      mongoClient.close();
    -    }
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        DistributedMode.cleanup();
    -      } else {
    -        SingleMode.cleanup();
    +      if (initCount.decrementAndGet() == 0) {
    +        try {
    +          if (mongoClient != null) {
    --- End diff --
    
    Then we will end up not to cleanup the cluster if mongoClient for some reason was set to null? I think it is better leave it as is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99451485
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -234,15 +243,25 @@ private static void createDbAndCollections(String dbName,
     
       @AfterClass
       public static void tearDownCluster() throws Exception {
    -    if (mongoClient != null) {
    -      mongoClient.dropDatabase(EMPLOYEE_DB);
    -      mongoClient.close();
    -    }
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        DistributedMode.cleanup();
    -      } else {
    -        SingleMode.cleanup();
    +      if (initCount.decrementAndGet() == 0) {
    +        try {
    +          if (mongoClient != null) {
    --- End diff --
    
    Move to top of function?
    ```
    if (mongoClient==null) { return; }
    if (initCount.decrementAndGet() == 0) { ...
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by gparai <gi...@git.apache.org>.
Github user gparai commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99470619
  
    --- Diff: contrib/storage-mongo/src/test/resources/datatype.json ---
    @@ -0,0 +1 @@
    +[{"_id": {"$oid" : "582081d96b69060001fd8938"}, "account" : { "$ref": "contact", "$id": {"$oid": "999cbf116b69060001fd8611"} }}, {"_id" : {"$oid" : "582081d96b69060001fd8939" } } ]
    --- End diff --
    
    Should we rename it to something like datatype_dbref.json? The name seems to suggest we have all datatypes in this document


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99451363
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -204,16 +209,20 @@ private static void cleanup() {
       @BeforeClass
       public static void initMongo() throws Exception {
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        logger.info("Executing tests in distributed mode");
    -        DistributedMode.setup();
    -      } else {
    -        logger.info("Executing tests in single mode");
    -        SingleMode.setup();
    +      if (initCount.get() == 0) {
    --- End diff --
    
    non-synchronized and if (initCount.getAndIncrement() == 0) ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #741: DRILL-5196: init MongoDB cluster when run a single test ca...

Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on the issue:

    https://github.com/apache/drill/pull/741
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99451419
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -234,15 +243,25 @@ private static void createDbAndCollections(String dbName,
     
       @AfterClass
       public static void tearDownCluster() throws Exception {
    -    if (mongoClient != null) {
    -      mongoClient.dropDatabase(EMPLOYEE_DB);
    -      mongoClient.close();
    -    }
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        DistributedMode.cleanup();
    -      } else {
    -        SingleMode.cleanup();
    +      if (initCount.decrementAndGet() == 0) {
    --- End diff --
    
    Because using the atomic int, also need the synchronized?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by chunhui-shi <gi...@git.apache.org>.
Github user chunhui-shi commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99452939
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -234,15 +243,25 @@ private static void createDbAndCollections(String dbName,
     
       @AfterClass
       public static void tearDownCluster() throws Exception {
    -    if (mongoClient != null) {
    -      mongoClient.dropDatabase(EMPLOYEE_DB);
    -      mongoClient.close();
    -    }
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        DistributedMode.cleanup();
    -      } else {
    -        SingleMode.cleanup();
    +      if (initCount.decrementAndGet() == 0) {
    --- End diff --
    
    Without Synchronized, there is still chance two threads go into this place. while thread 0 go to here and is in the middle of tearing down cluster, then thread 1 increases initCount to 1 and somehow report error then come to here to decrease initCount and continue... Then we don't know what will happen.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by chunhui-shi <gi...@git.apache.org>.
Github user chunhui-shi commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99455997
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/MongoTestSuit.java ---
    @@ -204,16 +209,20 @@ private static void cleanup() {
       @BeforeClass
       public static void initMongo() throws Exception {
         synchronized (MongoTestSuit.class) {
    -      if (distMode) {
    -        logger.info("Executing tests in distributed mode");
    -        DistributedMode.setup();
    -      } else {
    -        logger.info("Executing tests in single mode");
    -        SingleMode.setup();
    +      if (initCount.get() == 0) {
    --- End diff --
    
    Then it will increase the count when the init is not done. If init fail in the middle, then another test has no chance to try.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by gparai <gi...@git.apache.org>.
Github user gparai commented on a diff in the pull request:

    https://github.com/apache/drill/pull/741#discussion_r99470421
  
    --- Diff: contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/TestTableGenerator.java ---
    @@ -58,7 +59,12 @@ public static void generateTable(String dbName, String collection,
             .jsonArray(jsonArray).importFile(jsonFile).build();
         MongoImportExecutable importExecutable = MongoImportStarter
             .getDefaultInstance().prepare(mongoImportConfig);
    -    importExecutable.start();
    +    MongoImportProcess importProcess = importExecutable.start();
    +
    +    while (importProcess.isProcessRunning()) {
    --- End diff --
    
    Please add a comment explaining the change - Import process is running in a different thread/process and we do not to start running any test prior to import complete?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #741: DRILL-5196: init MongoDB cluster when run a single ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/drill/pull/741


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #741: DRILL-5196: init MongoDB cluster when run a single test ca...

Posted by gparai <gi...@git.apache.org>.
Github user gparai commented on the issue:

    https://github.com/apache/drill/pull/741
  
    +1 LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---