awslabs / amazon-dynamodb-lock-client

The AmazonDynamoDBLockClient is a general purpose distributed locking library built on top of DynamoDB. It supports both coarse-grained and fine-grained locking.
Other
476 stars 87 forks source link

NullPointerException #2

Closed scottwinkler closed 5 years ago

scottwinkler commented 7 years ago

I tried following the example for acquiring a lock from an item in dynamodb, but I am getting a NullPointerException, which seems to be caused by calling getS() on the String "ownerName" in the AmazonDynamoDBLockClient, at line 778. Below is a copy of my error message:

  "errorMessage": "java.lang.NullPointerException",
  "errorType": "java.lang.NullPointerException",
  "stackTrace": [
    "com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient.createLockItem(AmazonDynamoDBLockClient.java:778)",
    "com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient.getLockFromDynamoDB(AmazonDynamoDBLockClient.java:750)",
    "com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient.acquireLock(AmazonDynamoDBLockClient.java:402)",
    "com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient.tryAcquireLock(AmazonDynamoDBLockClient.java:567)",
    "com.elliemae.cpe.Hello.handleRequest(Hello.java:45)",
    "sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)",
    "sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)",
    "sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)",
    "java.lang.reflect.Method.invoke(Method.java:498)"
  ]
}

Here is my code that I am using, which is nearly identical to the example code.

        final boolean createHeartbeatBackgroundThread = true;
        //build the lock client
        final AmazonDynamoDBLockClient client = new AmazonDynamoDBLockClient(
                AmazonDynamoDBLockClientOptions.builder(ddbClient, "my-table")
                        .withTimeUnit(TimeUnit.SECONDS)
                        .withLeaseDuration(10L)
                        .withPartitionKeyName("key")
                        .withHeartbeatPeriod(3L)
                        .withCreateHeartbeatBackgroundThread(createHeartbeatBackgroundThread)
                        .build());

        //try to acquire a lock on the partition key "Moe"
        try {
            final Optional<LockItem> lockItem =
                    client.tryAcquireLock(AcquireLockOptions.builder("Moe").build());
            if (lockItem.isPresent()) {
                System.out.println("Acquired lock! If I die, my lock will expire in 10 seconds.");
                System.out.println("Otherwise, I will hold it until I stop heartbeating.");
                client.releaseLock(lockItem.get());
            } else {
                System.out.println("Failed to acquire lock!");
            }
            client.close();
        }
        catch(IOException | InterruptedException e){
            e.printStackTrace();
        }

The value of ownerName is definitely being set, as I used reflection to print out the value, but the problem seems to be with the getS() method. I do not think it matters, but I am using this library on aws lambda, not an ec2 instance.

skangayam commented 6 years ago

I'm running into the same exception. @ScottWinkler Were you able to find a workaround?

scottwinkler commented 6 years ago

@skangayam No, I was never able to get my code to work with this library. I decided to implement my own version of locking instead.

ajak6 commented 6 years ago

@scottwinkler what is the data in your table. Is it empty or it already has an entry for key Moe?

vineshroshan commented 6 years ago

Running into the same exception here.. final AttributeValue ownerName = item.remove(OWNER_NAME); final AttributeValue leaseDuration = item.remove(LEASE_DURATION); final AttributeValue recordVersionNumber = item.remove(RECORD_VERSION_NUMBER);

these lines inside the createLockItem method are trying to read the fields from the dynamodb item instead of the respective member variables that has the values already set. No sure if anyone could run this library that uses this method in the call stack.

ajak6 commented 6 years ago

@vineshroshan Does you table have these fields set? From my understanding Lock client assumes that it is managing the lock table exclusively. So if a record(primary key) exists, then it should be in the proper schema.

vineshroshan commented 6 years ago

@ajak6 thanks for your quick response; so am I correct in assuming that the lock client needs a separate table exclusively for locking and cannot use the existing tables (tried adding the columns
ownerName | recordVersionNumber | leaseDuration still doesnt work) ?

vineshroshan commented 6 years ago

@scottwinkler @skangayam @ajak6 the client works for an exclusive table, doesnt for an existing one

ajak6 commented 6 years ago

what value did you add in these columns? Whats the error? Please add stack trace. On Apr 24, 2018, at 4:45 PM, vineshroshan notifications@github.com<mailto:notifications@github.com> wrote:

@ajak6https://github.com/ajak6 thanks for your quick response; so am in correct in assuming that the lock client needs a separate table exclusively for locking and cannot use the existing tables (tried adding the columns ownerName | recordVersionNumber | leaseDuration still doesnt work) ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/awslabs/dynamodb-lock-client/issues/2#issuecomment-384114834, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIW7ovcCMWIznOSrjtOeoHtL89afDZlQks5tr7kcgaJpZM4PuZvN.

tracey-ruark commented 6 years ago

I'm also seeing this exact same null pointer. It seems to be related to the owner of the table, is this correct? Am I also reading the above comments correctly in that this client doesn't work with an existing table? It requires a new table that the client 'owns'?

skangayam commented 6 years ago

Hi @tracey-ruark I was able to get this to work as expected. Here is what i did. Let me know if it works.

Context: I wanted to lock a row in order to create a global distributed counter with row key "images"

  1. Create a separate lock table
  2. Bootstrap it with initial values
    private void bootstrap() {
        // check for the existence of the row with key "images"
        Map<String, AttributeValue> map = new HashMap<>();
        map.put("key", new AttributeValue().withS("images"));
        GetItemResult result = dynamoDB.getItem(new GetItemRequest(sequenceTableName, map));
        if (result.getItem() == null || result.getItem().isEmpty()) {
            buffer.putLong(0, counterStartValue);
            buffer.position(0);
            map.put("ownerName", new AttributeValue().withS("dummyOwner"));
            map.put("leaseDuration", new AttributeValue().withS("1"));
            map.put("recordVersionNumber", new AttributeValue().withS("dummyRecordVersionNumber"));
            map.put("data", new AttributeValue().withB(buffer));
            dynamoDB.putItem(new PutItemRequest(sequenceTableName, map));
        }
    }
  1. Use it as follows. (This part is documented)
    public long next() throws MyCustomExceptionClass {
        long result = 0;
        try {
            Optional<LockItem> lockItem = client.tryAcquireLock(AcquireLockOptions.builder("images")
                    .withDeleteLockOnRelease(false)
                    .withReplaceData(false)
                    .withAdditionalTimeToWaitForLock(additionalTimeToWaitForLockinSec)
                    .withTimeUnit(TimeUnit.SECONDS)
                    .build());
            if (lockItem.isPresent()) {
                result = lockItem.get().getData().get().getLong() + 1;
                buffer.putLong(0, result);
                buffer.position(0);
                ReleaseLockOptions releaseLockOptions = ReleaseLockOptions.builder(lockItem.get())
                        .withDeleteLock(false)
                        .withData(buffer)
                        .build();
                client.releaseLock(releaseLockOptions);
            } else {
                LOGGER.error("Failed to acquire lock!");
                throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_TIMEOUT_ERROR, "Could not acquire lock within the stipulated amount of time");
            }
        } catch (Exception e) {
            LOGGER.error("Failed to acquire lock. Exception {}", e);
            throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_FAILURE, "Exception while trying to acquire lock", e);
        }
        if (result == 0) {
            LOGGER.error("Failed to generate sequence");
            throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_DEFAULT_RESULT_ERROR, "Failed to generate sequence");
        }
        return result;
    }
tracey-ruark commented 6 years ago

Thanks for the quick reply @skangayam. So it appears that we do indeed need a separate table to keep track of locks. Hmmm, that really turns me off using this client. Having to duplicate and keep in sync the same data across two tables seems like a recipe for trouble. I'll adapt your implementation for my own needs just to see if I can get it working but I might try and solve my problem using other means.

ajak6 commented 6 years ago

Another workaround I did was to put dummy fields for ownerName in your table. AFAIR you will also need to out dummy values for lease duration, recordVersion number. Then it will work with your existing tables.

On Aug 27, 2018, at 9:01 AM, shashi kanth kangayam notifications@github.com<mailto:notifications@github.com> wrote:

Hi @tracey-ruarkhttps://github.com/tracey-ruark I was able to get this to work as expected. Here is what i did. Let me know if it works.

Context: I wanted to lock a row in order to create a global distributed counter with row key "images"

  1. Create a separate lock table

  2. Bootstrap it with initial values

    private void bootstrap() { // check for the existence of the row with key "images" Map<String, AttributeValue> map = new HashMap<>(); map.put("key", new AttributeValue().withS("images")); GetItemResult result = dynamoDB.getItem(new GetItemRequest(sequenceTableName, map)); if (result.getItem() == null || result.getItem().isEmpty()) { buffer.putLong(0, counterStartValue); buffer.position(0); map.put("ownerName", new AttributeValue().withS("dummyOwner")); map.put("leaseDuration", new AttributeValue().withS("1")); map.put("recordVersionNumber", new AttributeValue().withS("dummyRecordVersionNumber")); map.put("data", new AttributeValue().withB(buffer)); dynamoDB.putItem(new PutItemRequest(sequenceTableName, map)); } }

  3. Use it as follows. (This part is documented)

    public long next() throws MyCustomExceptionClass { long result = 0; try { Optional lockItem = client.tryAcquireLock(AcquireLockOptions.builder("images") .withDeleteLockOnRelease(false) .withReplaceData(false) .withAdditionalTimeToWaitForLock(additionalTimeToWaitForLockinSec) .withTimeUnit(TimeUnit.SECONDS) .build()); if (lockItem.isPresent()) { result = lockItem.get().getData().get().getLong() + 1; buffer.putLong(0, result); buffer.position(0); ReleaseLockOptions releaseLockOptions = ReleaseLockOptions.builder(lockItem.get()) .withDeleteLock(false) .withData(buffer) .build(); client.releaseLock(releaseLockOptions); } else { LOGGER.error("Failed to acquire lock!"); throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_TIMEOUT_ERROR, "Could not acquire lock within the stipulated amount of time"); } } catch (Exception e) { LOGGER.error("Failed to acquire lock. Exception {}", e); throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_FAILURE, "Exception while trying to acquire lock", e); } if (result == 0) { LOGGER.error("Failed to generate sequence"); throw new MyCustomExceptionClass(ExceptionCodes.DYNAMO_BACKED_SEQUENCE_DEFAULT_RESULT_ERROR, "Failed to generate sequence"); } return result; }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/awslabs/dynamodb-lock-client/issues/2#issuecomment-416276096, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIW7oqdF6LKkcM4FyISXJGCIwHKPX3Eoks5uVBfKgaJpZM4PuZvN.

skangayam commented 6 years ago

@tracey-ruark I was trying things out and resorted to the use of a separate table for ease of debugging. In practice however, you need not do that. The only requirement is to have the keys (ownerName, leaseDuration, recordVersionNumber) for the row you want to lock on. Additionally, in the releaseLockOptions you need to set the .withDeleteLock(false) to ensure that the bootstrapped values are not lost.

tracey-ruark commented 6 years ago

Thanks for the responses @skangayam and @ajak6. That definitely makes sense, I see what you mean. It could be done with an existing table. I might give that a shot when I continue to experiment with my implementation of this client. Thanks again.

It seems like the blog post that talks about this client needs an update. Setting up a separate table with those fields -or- adding those fields to an existing table should definitely be specified.

amcp commented 5 years ago

We have support for custom field names planned in the next release. To preserve referential integrity I think this particular best practice regarding using existing tables/data should be mentioned in a wiki here. I will work on that and have assigned myself an issue #20