Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NeedMoreData / Not enough data thrown randomly with amazon-dax-client #4038

Closed
4 tasks done
rchl opened this issue Feb 24, 2022 · 8 comments
Closed
4 tasks done

NeedMoreData / Not enough data thrown randomly with amazon-dax-client #4038

rchl opened this issue Feb 24, 2022 · 8 comments
Labels
bug This issue is a bug.

Comments

@rchl
Copy link

rchl commented Feb 24, 2022

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
I'm reporting this issue here even it concerns amazon-dax-client package as there is no other appropriate place to report it.

The issue happens randomly (in production) when using batchGetItem API to read from the Dax cluster.

Is the issue in the browser/Node.js?
Node.js

If on Node.js, are you running this on AWS Lambda?
No

Details of the browser/Node.js version
v14.18.1

SDK version number
2.1070.0

To Reproduce (observed behavior)
Difficult to provide as it doesn't happen reliably and requires access to Dax cluster.

The amazon-dax-client has this code that handles parsing of the Dax response:

exports.Custom_batchGetItem_N697851100_1_Assembler = class Custom_batchGetItem_N697851100_1_Assembler extends Assembler {
  _assembleResult() {
    // Assume the error response has already been handled
    let result = {};
    let length = this.dec.decodeArrayLength();
    if(length !== 2) {
      throw new DaxClientError('BatchGetResponse needs to have two elements, instead had: ' + arrHeader,
        DaxErrorCode.MalformedResult);
    }

    result.Responses = {};
    this.dec.processMap(() => {
      let tableName = this.dec.decodeString();
      let projOrdinals = this._request._tableProjOrdinals[tableName];
      let items = [];
      if(projOrdinals && projOrdinals.length > 0) {
        this.dec.processArray(() => {
          let item = this._decodeItemInternalHelper(projOrdinals);
          items.push(item);
        });
      } else {
        let keySchema = this._request._keysPerTable[tableName];
        let numItems = this.dec.decodeArrayLength();
        for(let i = 0; i < numItems; i += 2) {
          let key = Assembler._decodeKeyBytes(this.dec, keySchema);
          let item = Assembler._decodeStreamItem(this.dec.decodeCbor());
          item = Object.assign(item, key);

          items.push(item);
        }
      }

      result.Responses[tableName] = items;
    });

    result.UnprocessedKeys = {};
    this.dec.processMap(() => {
      let tableName = this.dec.decodeString();
      let keySchema = this._request._keysPerTable[tableName];
      let keys = this.dec.buildArray(() => Assembler._decodeKeyBytes(this.dec, keySchema));

      if(keys.length > 0) {
        let requestInfo = {Keys: keys};

        // Copy the RequestInfo back into the UnprocessedKeys items
        let tableRequest = this._request.RequestItems[tableName];
        // A table in the response should always be in the request, but guard just in case
        if(tableRequest) {
          for(let field of ['ProjectionExpression', 'ConsistentRead', 'AttributesToGet', 'ExpressionAttributeNames']) {
            let requestField = tableRequest[field];
            if(requestField) {
              requestInfo[field] = requestField;
            }
          }
        }

        result.UnprocessedKeys[tableName] = requestInfo;
      }
    });

    let consumedCapacities = this.dec.buildArray(() => Assembler._decodeConsumedCapacityData(this.dec.decodeCbor()));
    if(this._request.ReturnConsumedCapacity && this._request.ReturnConsumedCapacity !== 'NONE') {
      result.ConsumedCapacity = verifyBatchConsumedCapacity(consumedCapacities, Object.getOwnPropertyNames(this._request.RequestItems));
    }

    return result;
  }
};

The crash happens when trying to read the consumedCapacities from the response so near the end of this funciton. All the data before that is correctly parsed.

Is there are logic error in that code? Maybe it should check whether ReturnConsumedCapacity was requested before trying to parse the response further?

Though not sure why it wouldn't reproduce every time.

@rchl rchl added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 24, 2022
@ajredniwja
Copy link
Member

Hey @rchl thanks for opening issue, can you possibly check for any race conditions with your code?
To further investigate I would need something I can use to reproduce the issue.

@ajredniwja ajredniwja added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 8, 2022
@rchl
Copy link
Author

rchl commented Mar 8, 2022

Hi.

So after filing this I've also filed the #9647941061 case in AWS with more details and reproduction that I wouldn't be comfortable sharing there.

I got response that it will be followed up by DAX team so up to you how you want to handle this issue.

I could post more information here if that would be useful to you.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Mar 9, 2022
@kmcnerney-salesforce
Copy link

Hi, I'm seeing the same errors and have opened a ticket with AWS Support as well #9956481171. @rchl thanks for opening this, have you found any solution yet?

@rchl
Copy link
Author

rchl commented Apr 27, 2022

I'm using own version of the package with the following patch applied: rchl/amazon-dax-client@8ad2d19

It's a workaround specifically for the batchGetItem API so if your error triggers with some other API, it won't help you.

I haven't published that version of the package anywhere publicly.

@kmcnerney-salesforce
Copy link

Thanks for the quick response. One more question - since this code is just ignoring the error, do you know if the error is a real problem? Did AWS tell you that they acknowledge the problem and will work on a fix?

@rchl
Copy link
Author

rchl commented Apr 27, 2022

The problem is that this package in some circumstances stops reading from the network stream too early, before it received all data from DAX.

In the case of this specific API (batchGetItem) this means that it can stop reading the input before it received the ConsumedCapacity property.

If your batchGetItem request doesn't include the ReturnConsumedCapacity field and you don't care about the value of ConsumedCapacity then this workaround is entirely sufficient. But If you need the value of ConsumedCapacity then this workaround won't help you because the client will still randomly not deliver it. The only difference is that with the workaround it will be a silent failure (it will return all other data but not ConsumedCapacity).

AWS acknowledged it but didn't not provide any specific timeframe for a fix.

@kmcnerney-salesforce
Copy link

FYI: this issue is fixed in version amazon-dax-client 1.2.8

@rchl
Copy link
Author

rchl commented Jun 29, 2022

Have been using version with the fix for some time now and it appears to be fixed.

@rchl rchl closed this as completed Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants