Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-40577

Dropping a dataset with an non-existent dataverse returns error 24034 (cannot find dataverse) in 6.6, but a "cannot find dataset" error in 6.5

    XMLWordPrintable

Details

    • Untriaged
    • 1
    • Yes
    • CX Sprint 209

    Description

      We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.

      The test is:

      @Test
      void dropDatasetFailsIfAbsent() {
          assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo",
            dropDatasetAnalyticsOptions()
                .dataverseName("absentDataverse")));
      }
       

      In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code.  In this test and the one below too, neither "foo" nor "absentDataverse" exist.

      This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist.  Because that logic is expecting a DataverseNotFoundException. 

      E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed.  This test also passed in 6.5.

      @Test
      void dropDatasetCanIgnoreAbsent() {
        analytics.dropDataset("foo",
            dropDatasetAnalyticsOptions()
                .ignoreIfNotExists(true)
                .dataverseName("absentDataverse"));
      }
       

       For both tests, if I don't specify a dataverseName then the tests pass as expected.

      Attachments

        For Gerrit Dashboard: MB-40577
        # Subject Branch Project Status CR V

        Activity

          graham.pople Graham Pople created issue -
          graham.pople Graham Pople made changes -
          Field Original Value New Value
          Priority Minor [ 4 ] Major [ 3 ]
          graham.pople Graham Pople made changes -
          Description We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.

          The test is:
          {code:java}
          @Test
          void dropDatasetFailsIfAbsent() {
              assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo",
                dropDatasetAnalyticsOptions()
                    .dataverseName("absentDataverse")));
          }
           {code}
          In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code.

          This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist.  Because that logic is expecting a DataverseNotFoundException. 

          E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed (neither "foo" nor "absentDataverse" exist).  This test also passed in 6.5.
          {code:java}
          @Test
          void dropDatasetCanIgnoreAbsent() {
            analytics.dropDataset("foo",
                dropDatasetAnalyticsOptions()
                    .ignoreIfNotExists(true)
                    .dataverseName("absentDataverse"));
          }
           {code}
           
          We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.

          The test is:
          {code:java}@Test
          void dropDatasetFailsIfAbsent() {
              assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo",
                dropDatasetAnalyticsOptions()
                    .dataverseName("absentDataverse")));
          }
           {code}
          In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code.  In this test and the one below too, neither "foo" nor "absentDataverse" exist.

          This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist.  Because that logic is expecting a DataverseNotFoundException. 

          E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed.  This test also passed in 6.5.
          {code:java}@Test
          void dropDatasetCanIgnoreAbsent() {
            analytics.dropDataset("foo",
                dropDatasetAnalyticsOptions()
                    .ignoreIfNotExists(true)
                    .dataverseName("absentDataverse"));
          }
           {code}
           For both tests, if I don't specify a dataverseName then the tests pass as expected.
          till Till Westmann made changes -
          Rank Ranked higher
          till Till Westmann made changes -
          Affects Version/s 6.6.0 [ 16787 ]
          till Till Westmann made changes -
          Labels triaged
          till Till Westmann made changes -
          Assignee Till Westmann [ till ] Dmitry Lychagin [ dmitry.lychagin ]

          This behavior was changed by https://issues.couchbase.com/browse/MB-39142 , specifically:
          https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6203/8/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java#1576

          Graham Pople,

          We discussed this issue today and believe the new error code better explains the problem to the end user because it's more specific ("dataverse not found" instead of "dataset not found"). 

          Could you consider fixing it on your side?  If ignoreIfNotExists=true then do not fail on either DataverseNotFoundException or DatasetNotFoundException?

          dmitry.lychagin Dmitry Lychagin (Inactive) added a comment - This behavior was changed by  https://issues.couchbase.com/browse/MB-39142  , specifically: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6203/8/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java#1576 Graham Pople , We discussed this issue today and believe the new error code better explains the problem to the end user because it's more specific ("dataverse not found" instead of "dataset not found").  Could you consider fixing it on your side?  If ignoreIfNotExists=true then do not fail on either DataverseNotFoundException or DatasetNotFoundException?
          graham.pople Graham Pople added a comment -

          Dmitry, thanks for the reply.  I maybe agree with you on the specificity being better, but it is awkward for us to fix our side due to the changes needing to go into multiple SDKs, plus we need to update our sdk-rfcs detailing the protocol.  I will raise with the team and get back to you.

          graham.pople Graham Pople added a comment - Dmitry, thanks for the reply.  I maybe agree with you on the specificity being better, but it is awkward for us to fix our side due to the changes needing to go into multiple SDKs, plus we need to update our sdk-rfcs detailing the protocol.  I will raise with the team and get back to you.
          graham.pople Graham Pople added a comment -

          Actually, I need to revisit the problem description.  I think there are two separate problems here.  Problem 1 is that this code:

          analytics.dropDataset("absentDataset", dropDatasetAnalyticsOptions().dataverseName("absentDataverse")) 

          used to raise DatasetNotFoundException and now raises DataverseNotFoundException. 

          I saw the ignoreIfNotExists test was failing also and assumed it was caused by the same change.  I'm rusty on the analytics manager code and thought it worked the same as N1QL, where we implement ignoreIfNotExists by ignoring a specific error code.  But analytics is different, it supports "IF EXISTS" syntax here, which we use. 

          So in fact I think there is a separate problem/regression 2 here, where

          analytics.dropDataset("absentDataset", dropDatasetAnalyticsOptions().ignoreIfNotExists().dataverseName("absentDataverse")) 

          which results in a single statement with "IF EXISTS" being sent to cbas, used to return success but now returns an error code causing us to raise DataverseNotFoundException. 

          If all that makes sense?

           

          So to clarify, the ask from analytics team is that all SDKs:

          1. For problem 1: I'm not sure?  Make no change and just accept that this is a minor breaking change for users in this edge case?

          2. For problem 2: ignore the 24034 (dataverse not found) error code if doing a "DROP DATASET ... IF EXISTS" statement?

          graham.pople Graham Pople added a comment - Actually, I need to revisit the problem description.  I think there are two separate problems here.  Problem 1 is that this code: analytics.dropDataset( "absentDataset" , dropDatasetAnalyticsOptions().dataverseName( "absentDataverse" )) used to raise DatasetNotFoundException and now raises DataverseNotFoundException.  I saw the ignoreIfNotExists test was failing also and assumed it was caused by the same change.  I'm rusty on the analytics manager code and thought it worked the same as N1QL, where we implement ignoreIfNotExists by ignoring a specific error code.  But analytics is different, it supports "IF EXISTS" syntax here, which we use.  So in fact I think there is a separate problem/regression 2 here, where analytics.dropDataset( "absentDataset" , dropDatasetAnalyticsOptions().ignoreIfNotExists().dataverseName( "absentDataverse" )) which results in a single statement with "IF EXISTS" being sent to cbas, used to return success but now returns an error code causing us to raise DataverseNotFoundException.  If all that makes sense?   So to clarify, the ask from analytics team is that all SDKs: 1. For problem 1: I'm not sure?  Make no change and just accept that this is a minor breaking change for users in this edge case? 2. For problem 2: ignore the 24034 (dataverse not found) error code if doing a "DROP DATASET ... IF EXISTS" statement?
          till Till Westmann made changes -
          Fix Version/s 6.6.0 [ 16787 ]
          till Till Westmann made changes -
          Is this a Regression? Unknown [ 10452 ] Yes [ 10450 ]
          dmitry.lychagin Dmitry Lychagin (Inactive) made changes -
          Status Open [ 1 ] In Progress [ 3 ]

          Graham Pople,

          The change in IF EXISTS behavior (problem 2) is a regression. We're working on fixing this in 6.6.

          As for problem 1, our proposal is to leave it as is. So if user does not request "ignoreIfNotExists" and the dataverse does not exist, then the user will get DataverseNotFoundException instead of previous DatasetNotFoundException.

          dmitry.lychagin Dmitry Lychagin (Inactive) added a comment - Graham Pople , The change in IF EXISTS behavior (problem 2) is a regression. We're working on fixing this in 6.6. As for problem 1, our proposal is to leave it as is. So if user does not request "ignoreIfNotExists" and the dataverse does not exist, then the user will get DataverseNotFoundException instead of previous DatasetNotFoundException.
          till Till Westmann made changes -
          Link This issue blocks MB-38724 [ MB-38724 ]
          till Till Westmann made changes -
          Labels triaged approved-for-6.6.0 triaged
          till Till Westmann made changes -
          Due Date 24/Jul/20
          dmitry.lychagin Dmitry Lychagin (Inactive) made changes -
          Sprint CX Sprint 209 [ 1167 ]
          dmitry.lychagin Dmitry Lychagin (Inactive) made changes -
          Link This issue relates to MB-40592 [ MB-40592 ]
          graham.pople Graham Pople added a comment -

          Ok Dmitry Lychagin, I'll let the team know and let you know if they have any feedback.  That sounds ok to me though, I'll modify these tests accordingly.  MB-40576 is more of a problem as we'll have to find some way of disabling the analytics tests, or a subset of them, but only on 6.6.0.

          graham.pople Graham Pople added a comment - Ok Dmitry Lychagin , I'll let the team know and let you know if they have any feedback.  That sounds ok to me though, I'll modify these tests accordingly.   MB-40576 is more of a problem as we'll have to find some way of disabling the analytics tests, or a subset of them, but only on 6.6.0.
          dmitry.lychagin Dmitry Lychagin (Inactive) made changes -
          Resolution Fixed [ 1 ]
          Status In Progress [ 3 ] Resolved [ 5 ]

          Build couchbase-server-6.6.0-7896 contains cbas-core commit c23928d with commit message:
          MB-40577: DROP DATASET IF EXISTS must not fail if dataverse not found

          build-team Couchbase Build Team added a comment - Build couchbase-server-6.6.0-7896 contains cbas-core commit c23928d with commit message: MB-40577 : DROP DATASET IF EXISTS must not fail if dataverse not found
          ritam.sharma Ritam Sharma made changes -
          Assignee Dmitry Lychagin [ dmitry.lychagin ] Arunkumar Senthilnathan [ arunkumar ]
          arunkumar Arunkumar Senthilnathan made changes -
          Labels approved-for-6.6.0 triaged approved-for-6.6.0 combination_testing triaged
          arunkumar Arunkumar Senthilnathan made changes -
          Assignee Arunkumar Senthilnathan [ arunkumar ] Will Broadbelt [ will.broadbelt ]

          Verified tests pass with 6.6.0-7897.

          will.broadbelt Will Broadbelt added a comment - Verified tests pass with 6.6.0-7897.
          will.broadbelt Will Broadbelt made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

          Build couchbase-server-7.0.0-2746 contains cbas-core commit c23928d with commit message:
          MB-40577: DROP DATASET IF EXISTS must not fail if dataverse not found

          build-team Couchbase Build Team added a comment - Build couchbase-server-7.0.0-2746 contains cbas-core commit c23928d with commit message: MB-40577 : DROP DATASET IF EXISTS must not fail if dataverse not found

          Build couchbase-server-6.6.2-9599 contains cbas-core commit c23928d with commit message:
          MB-40577: DROP DATASET IF EXISTS must not fail if dataverse not found

          build-team Couchbase Build Team added a comment - Build couchbase-server-6.6.2-9599 contains cbas-core commit c23928d with commit message: MB-40577 : DROP DATASET IF EXISTS must not fail if dataverse not found

          People

            will.broadbelt Will Broadbelt
            graham.pople Graham Pople
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty