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

Rate limited Scan Implementation. #205

Merged
merged 11 commits into from Dec 21, 2016
Merged

Rate limited Scan Implementation. #205

merged 11 commits into from Dec 21, 2016

Conversation

anandswaminathan
Copy link
Contributor

@anandswaminathan anandswaminathan commented Nov 30, 2016

The rate limited scan tries to pace the scan speed using sleeps, and also handles provision throughput exceeded exception. In order to handle large item sizes, the application can use the page_size variable to pace the scan if needed.

@danielhochman

@anandswaminathan anandswaminathan changed the title STRUCTURE REVIEW - Rate limited Scan. NO TESTS Pseudo code - Rate limited Scan. NO TESTS Nov 30, 2016
@anandswaminathan anandswaminathan changed the title Pseudo code - Rate limited Scan. NO TESTS Code for Rate limited Scan. [TESTS Pending] Dec 2, 2016
@coveralls
Copy link

coveralls commented Dec 2, 2016

Coverage Status

Coverage decreased (-1.1%) to 96.639% when pulling 4e8deec on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@anandswaminathan anandswaminathan changed the title Code for Rate limited Scan. [TESTS Pending] Code for Rate limited Scan with tests Dec 5, 2016
@coveralls
Copy link

coveralls commented Dec 5, 2016

Coverage Status

Coverage decreased (-0.4%) to 97.409% when pulling 2fad33b on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

# throttled_scan call.
elapsed_time_ms = round((current_time - start_time) * 1000)
# elapsed_time_ms can be 0 or negative if there is a clock drift
if elapsed_time_ms < 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could collapse this to single line on :1009 using max()


if consecutive_provision_throughput_exceeded_ex == 0:
total_consumed_read_capacity += latest_scan_consumed_capacity
consumed_rate = total_consumed_read_capacity / elapsed_time_ms
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would probably from __future__ import division at top so you don't need to worry about accidentally doing integer division

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. Awesome this is. Never knew it existed.

@coveralls
Copy link

coveralls commented Dec 5, 2016

Coverage Status

Coverage decreased (-0.2%) to 97.577% when pulling 8df40c8 on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@coveralls
Copy link

coveralls commented Dec 6, 2016

Coverage Status

Coverage decreased (-0.01%) to 97.747% when pulling 405a91f on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@coveralls
Copy link

coveralls commented Dec 6, 2016

Coverage Status

Coverage decreased (-0.01%) to 97.747% when pulling 671b933 on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

exclusive_start_key=None,
segment=None,
total_segments=None,
model_cls=None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the model should never make its way into the connection

@coveralls
Copy link

coveralls commented Dec 8, 2016

Coverage Status

Coverage increased (+0.07%) to 97.831% when pulling a344bbf on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@anandswaminathan anandswaminathan changed the title Code for Rate limited Scan with tests Rate limited Scan Implementation. Dec 16, 2016
@coveralls
Copy link

coveralls commented Dec 16, 2016

Coverage Status

Coverage increased (+0.07%) to 97.83% when pulling 90b2999 on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

if code == "ProvisionedThroughputExceededException":
consecutive_provision_throughput_exceeded_ex += 1
if consecutive_provision_throughput_exceeded_ex > max_consecutive_exceptions:
raise # Max threshold reached
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pep8 says two space before inline comment

i would put them on line above the raise though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for below two comments

elapsed_time_s = math.ceil(elapsed_time_ms / 1000)
# Sleep proportional to the ratio of --consumed capacity-- to --capacity to consume--
time_to_sleep = max(1, round((total_consumed_read_capacity/ elapsed_time_s) \
/ (read_capacity_to_consume_per_second)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra set of parenthesis around read_capacity_to_consume_per_second

@@ -131,6 +131,37 @@ def get_item(self, hash_key, range_key=None, consistent_read=False, attributes_t
consistent_read=consistent_read,
attributes_to_get=attributes_to_get)

def rate_limited_scan(self,
attributes_to_get=None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either move self down a line or indent everything to same level as self

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey. I am following the format that was present throughout the file. Making this change will make this method appear odd

max_sleep_between_retry=max_sleep_between_retry,
max_consecutive_exceptions=max_consecutive_exceptions,
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra blank line

@coveralls
Copy link

coveralls commented Dec 20, 2016

Coverage Status

Coverage increased (+0.07%) to 97.83% when pulling cc0f766 on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@coveralls
Copy link

coveralls commented Dec 21, 2016

Coverage Status

Coverage increased (+0.07%) to 97.83% when pulling f80ad89 on lyft:THROTTLED-SCAN into cd705cc on jlafon:devel.

@danielhochman danielhochman merged commit 4a808b7 into pynamodb:devel Dec 21, 2016
@danielhochman danielhochman deleted the THROTTLED-SCAN branch December 21, 2016 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants