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

alternator: maybe support StreamSpecification: null #5796

Closed
dimaqq opened this issue Feb 12, 2020 · 6 comments
Closed

alternator: maybe support StreamSpecification: null #5796

dimaqq opened this issue Feb 12, 2020 · 6 comments
Assignees
Labels
area/alternator Alternator related Issues

Comments

@dimaqq
Copy link

dimaqq commented Feb 12, 2020

I'm testing out scylladb+alternator.
I've figured to swap dynalite with scylla in our unit tests first.

I'm hitting this error:
https://github.com/scylladb/scylla/blob/a8a4e584ec3332067b6b9e1756ba1bf2392e1424/alternator/executor.cc#L807-L809

I've tracked it down to some library in our stack passing None / null to the StreamSpecification when creating a table.
The intention is to use a default, i.e. same as if the StreamSpecification was not there at all.
It seems that AWS SaaS dynamodb doesn't make a distinction.
dynamodb-local doesn't appear to make a distinction.
dynalite doesn't care.

Maybe Scylla should also ignore StreamSpecification: null

Installation details
Scylla version (or git commit hash): 3.3.rc1-0.20200209.0d0c1d43188
Cluster size: 🤷‍♂️
OS (RHEL/CentOS/Ubuntu/AWS AMI): Docker scylladb/scylla

@nyh nyh self-assigned this Feb 12, 2020
@nyh nyh added the area/alternator Alternator related Issues label Feb 12, 2020
@nyh
Copy link
Contributor

nyh commented Feb 12, 2020

I'm having trouble reproducing this in boto3 in Python, trying to pass StreamSpecification=None doesn't pass boto3's internal checks:

E               ParamValidationError: Parameter validation failed:
E               Invalid type for parameter StreamSpecification, value: None, type: <type 'NoneType'>, valid types: <type 'dict'>

If I try to pass StreamSpecification={}, DynamoDB doesn't allow this either.
Anyway, I'll prepare a patch for you to test, and test what I can in the boto3-based tests.

@nyh
Copy link
Contributor

nyh commented Feb 12, 2020

I just sent a patch titled "Alternator: allow CreateTable with streams explicitly turned off" to the mailing list, which should hopefully fix this issue (if you can, please test it).
With that patch, if StreamSpecification is not a map (e.g., a null as in your case) or if it has StreamEnabled=false, then table creation is allowed. Only if StreamEnabled=true is explicitly set, do we fail the table creation.

@dimaqq
Copy link
Author

dimaqq commented Apr 1, 2020

I'm having trouble reproducing this in boto3 in Python, trying to pass StreamSpecification=None doesn't pass boto3's internal checks:

E               ParamValidationError: Parameter validation failed:
E               Invalid type for parameter StreamSpecification, value: None, type: <type 'NoneType'>, valid types: <type 'dict'>

If I try to pass StreamSpecification={}, DynamoDB doesn't allow this either.
Anyway, I'll prepare a patch for you to test, and test what I can in the boto3-based tests.

We use another library, https://github.com/HENNGE/aiodynamo which is notably faster :)
The divergence from boto* was fixed there too, just in case.

@dorlaor
Copy link
Contributor

dorlaor commented Apr 1, 2020 via email

@ojii
Copy link

ojii commented Apr 1, 2020

Can you please share performance numbers, I see there is a performance section in the docs but no actual results.

Some results can be found in this comment, they were generated with the code found here.

TL;DR: For query/scan (especially of many items), aiodynamo is about 2x faster than boto based libraries.

@nyh
Copy link
Contributor

nyh commented Apr 19, 2020

Already in 4.0, not backporting to 3.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alternator Alternator related Issues
Projects
None yet
Development

No branches or pull requests

5 participants