Uploaded image for project: 'Couchbase Java Client'
  1. Couchbase Java Client
  2. JCBC-45

asyncGetAndLock returns the same value when a lock is refused and when a key is not found

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.2
    • Fix Version/s: 1.0.3
    • Component/s: Core
    • Security Level: Public
    • Labels:

      Description

      This report is re: asyncGetAndLock method in the java client.

      From the current implementation of asyncGetAndLock , it is impossible to tell the difference between

      • a value that doesn't exist(& should be added) and
      • a value that exists, but is locked by another client.

      In both cases, you get this:
      CASValue<T>(-1, null);

      which customer opines is pretty non-useful in the cases where you want to decide whether to try again or move on to an add.

      Suggestion from the customer is:

      Add the CasGetStatus class which has additional information about the failure type (attached), and modify CouchbaseClient.java to create and return that class (diff attached).

      Attached files:
      1) CASGetStatus.java
      2) couchbasediff.txt

      1. CASGetStatus.java
        0.9 kB
        Hari Subramaniam
      2. couchbasediff.txt
        5 kB
        Hari Subramaniam
      No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

        Hide
        ingenthr Matt Ingenthron added a comment -

        Not sure about that implementation, but need acknowledged.

        Show
        ingenthr Matt Ingenthron added a comment - Not sure about that implementation, but need acknowledged.
        Hide
        ingenthr Matt Ingenthron added a comment -

        Also, note that code contributions can be posted over at http://review.couchbase.org. Thanks!

        Show
        ingenthr Matt Ingenthron added a comment - Also, note that code contributions can be posted over at http://review.couchbase.org . Thanks!
        Hide
        ingenthr Matt Ingenthron added a comment -

        (from Mike)

        The information that you are trying to get from the asyncGetAndLock() function can already be obtained by calling getStatus() on the Future object that is returned. See my example below:

        Future<CasValue<Object>> future = client.asyncGetAndLock("key");
        future.getStatus().getMessage();
        // If key is locked returns "locked"
        // If key is not found returns "not found"
        // If key is timed out returns "timed out"

        Can you try using this method? If you find any shortcomings or feel that your approach provides a major improvement to this then please let me know.

        • Mike
        Show
        ingenthr Matt Ingenthron added a comment - (from Mike) The information that you are trying to get from the asyncGetAndLock() function can already be obtained by calling getStatus() on the Future object that is returned. See my example below: Future<CasValue<Object>> future = client.asyncGetAndLock("key"); future.getStatus().getMessage(); // If key is locked returns "locked" // If key is not found returns "not found" // If key is timed out returns "timed out" Can you try using this method? If you find any shortcomings or feel that your approach provides a major improvement to this then please let me know. Mike
        Hide
        hari Hari Subramaniam (Inactive) added a comment -

        Mike:

        Thanks for the quick response. While I understand your suggestion on how to get at the exact status by introspecting the returned Future object, I received an understandable response back from the customer - i.e. the API return code should indicate the accurate return status so the program logic can make runtime decisions without having to parse the string returned by getstatus()..Here are the customers' comments (unedited):
        -------------------------------------------------------------------------------------------------------------------
        I need the status so I can make run time decisions about whether to retry the request (in the case that it is locked, and will likely be unlocked by the time the retry happens) or to add the object (in the case that it doesn't exist).

        The OperationFuture's status is fine for logging, but of course we don't want to make run time decisions based on string equality.
        -------------------------------------------------------------------------------------------------------------------

        sample snippet from the customer:
        CASValue<T> rawCas =
        this.memcacheClient.getAndLock(key.getRaw(),
        getOperationTimeout().intValue(), trans);
        Assert.isTrue(rawCas instanceof CASGetStatus);
        CASGetStatus<T> cas = (CASGetStatus<T>) rawCas;

        if (cas.getGetResponse() == CASGetResponse.NOT_FOUND)

        { return null; } else if (cas.getGetResponse() != CASGetResponse.OK) { throw new CASReadException( "Failed to get an object due to a timeout", cas.getGetResponse()); }

        To do that with the current system, I'd have to do something sketchy like:
        If(future.getStatus().getMessage().equals("not found")) { return null; }

        else …

        Hope that helps. Any further comments on the suggested change? Thanks!

        Show
        hari Hari Subramaniam (Inactive) added a comment - Mike: Thanks for the quick response. While I understand your suggestion on how to get at the exact status by introspecting the returned Future object, I received an understandable response back from the customer - i.e. the API return code should indicate the accurate return status so the program logic can make runtime decisions without having to parse the string returned by getstatus()..Here are the customers' comments (unedited): ------------------------------------------------------------------------------------------------------------------- I need the status so I can make run time decisions about whether to retry the request (in the case that it is locked, and will likely be unlocked by the time the retry happens) or to add the object (in the case that it doesn't exist). The OperationFuture's status is fine for logging, but of course we don't want to make run time decisions based on string equality. ------------------------------------------------------------------------------------------------------------------- sample snippet from the customer: CASValue<T> rawCas = this.memcacheClient.getAndLock(key.getRaw(), getOperationTimeout().intValue(), trans); Assert.isTrue(rawCas instanceof CASGetStatus); CASGetStatus<T> cas = (CASGetStatus<T>) rawCas; if (cas.getGetResponse() == CASGetResponse.NOT_FOUND) { return null; } else if (cas.getGetResponse() != CASGetResponse.OK) { throw new CASReadException( "Failed to get an object due to a timeout", cas.getGetResponse()); } To do that with the current system, I'd have to do something sketchy like: If(future.getStatus().getMessage().equals("not found")) { return null; } else … Hope that helps. Any further comments on the suggested change? Thanks!
        Hide
        mikew Mike Wiederhold added a comment -

        Turns out I just committed some code to review that allows the customer to do this. The commits are below. I will close this bug once it gets through review.

        http://review.couchbase.org/#change,15948
        http://review.couchbase.org/#change,15949

        Show
        mikew Mike Wiederhold added a comment - Turns out I just committed some code to review that allows the customer to do this. The commits are below. I will close this bug once it gets through review. http://review.couchbase.org/#change,15948 http://review.couchbase.org/#change,15949
        Hide
        mikew Mike Wiederhold added a comment -

        This change that were just committed will allow the user to get the error code returned by all operations. This can be done by calling getStatus().getErrorCode() on any Future returned to the user through CouchbaseClient. Error codes are kept in the ErrorCode enum in the net.spy.memcached.op.ErrorCode class. Please inquire with either myself or Matt for more information.

        Show
        mikew Mike Wiederhold added a comment - This change that were just committed will allow the user to get the error code returned by all operations. This can be done by calling getStatus().getErrorCode() on any Future returned to the user through CouchbaseClient. Error codes are kept in the ErrorCode enum in the net.spy.memcached.op.ErrorCode class. Please inquire with either myself or Matt for more information.

          People

          • Assignee:
            mikew Mike Wiederhold
            Reporter:
            hari Hari Subramaniam (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Gerrit Reviews

              There are no open Gerrit changes