BuildableSearch#complete!.to_a auto-paginates silently and Limit is per-page (companion to #153)
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 25/100
- Loại issue
- Tính năng
- Độ rõ ràng
- Khá rõ ràng
- Mức độ hoạt động
- Ít trao đổi
- Công nghệ
- aws, ruby
- Lĩnh vực
- backend-api-design, databases
Hướng nghiên cứu
Bắt đầu bằng cách đọc Aws::Record::BuildableSearch#complete! và Aws::Record::ItemCollection#to_a cùng #each, sau đó kiểm tra các specs hiện có về hành vi phân trang và lặp. Công việc bao gồm tài liệu, các phương thức kết thúc tường minh và một chế độ lặp an toàn; công việc được xem là hoàn tất khi các contract được yêu cầu, hành vi khi thất bại và các giới hạn phân trang đã được triển khai và được kiểm thử.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Describe the feature
Two coordinated improvements to Aws::Record::BuildableSearch and Aws::Record::ItemCollection, motivated by a real-world refactor we just shipped:
-
Documentation that clarifies (a)
BuildableSearch#limitis per-request page size, NOT a total cap; (b) iterating an
ItemCollection(via#to_a,#each, or any other Enumerable method) auto-paginates the entire query. -
Strategy-explicit terminals on
BuildableSearchas first-class methods, replacing the implicit "what does.complete!.to_amean for cost?" question with explicit verbs: a single-page method, a take-N method, and an exhaust-with-cap method. Concrete reference implementation included below. -
A safe iteration mode that surfaces transient errors as inspectable Result objects instead of raising mid-walk - companion to #153, which has been sitting on this exact reliability concern for 5 months.
Use Case
I'm always frustrated when innocent-looking code silently issues many more DynamoDB requests than the author intended.
A pattern that appears across our codebase (and the community — see #39 from 7 years ago, where the same misunderstanding was raised and closed without a doc fix):
records = build_query
.on_index(:tenant_and_key_gsi)
.key_expr(':tenant_and_key = ? AND :range_value = ?', partition, value)
.limit(1)
.complete!
.to_a
records.first
Reads as: "fetch one record by GSI key." Actually does: "fetch in pages of 1, walking every page until the partition is exhausted."
BuildableSearch#limit(n) sets DynamoDB's Limit parameter (per-request page size), not a total cap. Combined with ItemCollection#to_a silently auto-paginating, callers consistently get this wrong. Our internal audit found ~26 .limit(n) call sites where the author trusted limit as a total cap; several were latent unbounded reads waiting for the data shape to grow.
The deeper issue is that complete! returns an ItemCollection whose iteration semantics (one DynamoDB request? all pages? what happens on mid-walk failure?) aren't visible at the call site. The verb complete! tells you the query is built, not how iterating its result will fan out into DynamoDB requests. Every caller has to remember the contract.
This is also the companion to #153: auto-pagination amplifies iteration fragility because every additional page is another chance for transient throttling or a network blip mid-walk. Five months after we filed #153, our codebase still wraps #to_a/#each in defensive rescue StandardError — because the underlying contract is still fuzzy.
Proposed Solution
We just shipped a refactor that introduces three explicit terminals on Aws::Record::BuildableSearch via Module#prepend, migrating ~24 call sites away from the ambiguous .complete!.to_a pattern. Our reference iplementation in production today:
module Patches
module AwsRecordBuildableSearchSafeTerminals
TERMINALS = %i[to_a_page to_a_take to_a_exhaust!].freeze
def self.install!
require 'aws-record'
collisions = TERMINALS.select do |method_name|
Aws::Record::BuildableSearch.method_defined?(method_name) ||
Aws::Record::BuildableSearch.private_method_defined?(method_name)
end
if collisions.any? && (Aws::Record::BuildableSearch.ancestors & [self]).empty?
raise "Aws::Record::BuildableSearch already defines #{collisions.join(', ')}; " \
're-evaluate the safe terminal patch.'
end
Aws::Record::BuildableSearch.prepend(self) unless
Aws::Record::BuildableSearch.ancestors.include?(self)
end
# Single DynamoDB request. Returns first page only.
def to_a_page
result = complete!
return [] if result.nil?
return result.page || [] if result.is_a?(Aws::Record::ItemCollection)
terminal_array(result)
rescue StandardError => e
log_terminal_failure(:to_a_page, e)
[]
end
# Walk pages collecting up to `count` records. Pushes
# min(existing_limit, count) into the per-request Limit so
# the gem doesn't over-fetch the first page.
def to_a_take(count)
count_int = count.respond_to?(:to_i) ? count.to_i : 0
return [] if count_int <= 0
@params[:limit] = [@params[:limit], count_int].compact.min
result = complete!
return [] if result.nil?
return terminal_array(result).first(count_int) unless
result.is_a?(Aws::Record::ItemCollection)
collected = []
result.each do |item|
collected << item
break if collected.size >= count_int
end
collected
rescue StandardError => e
log_terminal_failure(:to_a_take, e)
[]
end
# Walk every page; raise once the cap is exceeded.
# `max: nil` opts out of the cap.
def to_a_exhaust!(max: 1_000)
result = complete!
return [] if result.nil?
collected = []
enumerable = result.is_a?(Aws::Record::ItemCollection) ? result : terminal_array(result)
enumerable.each do |item|
collected << item
if max && collected.size > max
raise ResultSetTooLargeError.new(model: model_name, max_items: max)
end
end
collected
end
private
def terminal_array(result)
result.is_a?(Array) ? result : [result]
end
def log_terminal_failure(method_name, error)
Rails.logger&.warn("AwsRecordBuildableSearchSafeTerminals##{method_name} failed:
#{error.class}: #{error.message}")
end
def model_name
instance_variable_get(:@model)&.name
end
end
end
How call sites changed
Single-record GSI lookup — was implicit, now explicit:
# before
records = wrap_records(
build_query.on_index(:tenant_and_key_gsi).key_expr(...).limit(1).complete!,
single_page: true,
)
records.first
# after
records = build_query.on_index(:tenant_and_key_gsi).key_expr(...).to_a_page
records.first
Duplicate detection — was silently auto-paginating, now bounded:
# before
results = wrap_records(build_query.on_index(:class_gsi).key_expr(...).complete!)
raise DuplicateSlug if results.count > 1
# after
results = build_query.on_index(:class_gsi).key_expr(...).to_a_take(2)
raise DuplicateSlug if results.count > 1
Full enumeration with safety cap — was open-ended each, now loud failure mode:
# before
base_query.complete!.each do |record|
next unless record.version.zero?
results << record
end
# after
base_query
.filter_expr('version = ?', 0)
.to_a_exhaust!(max: 1_000)
Asks (priority order, all first-class)
- Loud rdoc warnings on
ItemCollection#to_a,#each, andBuildableSearch#complete!clarifying that iteration auto-paginates AND may raise mid-walk on transient AWS errors. - Clarify
BuildableSearch#limitrdoc to state that it is per-request, not total. Even one sentence would prevent a class of bugs. - Document
#to_afailure modes — when nil vs empty Array, what happens to already-yielded items on mid-walk failure, the retry contract for transient errors. - Adopt strategy-explicit terminals on
BuildableSearchalong the lines of the reference implementation above.complete!could remain for cursor-aware callers; the new terminals would be the primary path for everyone else. - A safe iteration mode that returns inspectable Result objects (yielded items + error + continuation token) instead of raising mid-walk. Companion to #153.
Other Information
Real-world impact. Before this refactor, our codebase had
~26 .limit(n) call sites trusting limit as a total cap that
the gem doesn't actually provide, plus several latent
unbounded-pagination paths that iterated .complete!.to_a
directly. The migration touched ~50 files and removed our
defensive wrap_records helper entirely.
Spec coverage of the reference implementation. 10 RSpec
examples covering: single-page semantics, n <= 0 early return,
per-request Limit pushdown, min(existing_limit, n)
precedence, multi-page bounded collect, max: boundary raise,
max: nil unbounded path, non-ItemCollection Hash/Struct
fallback, and a load-time collision guard. Test fake walks real
multi-page data and tracks page-walk count to verify pagination
efficiency.
Why we still need defensive rescue around to_a. In our
implementation, to_a_page and to_a_take wrap their work in
rescue StandardError → log + return [] because #to_a and
#each on ItemCollection still raise unpredictably (the
exact symptom #153 was filed about, 5 months ago). We could not
delete the defensive rescue; the gem's contract is still too
fuzzy. This is fresh evidence that #153 remains live.
Related issues:
- #153 —
Aws::Record::ItemCollectionintermittently returns
nil/raises fromto_aandeach(filed by me, 5 months ago,
still untriaged; refreshed with the same
Patches::AwsRecordBuildableSearchSafeTerminalsreference
implementation today). - #39 — "Is query limit supported?" (closed 7 years ago, same
root misunderstanding aboutLimitsemantics).
Acknowledgements
- I may be able to implement this feature request
- This feature might incur a breaking change
aws-sdk-ruby-record version used
latest
Environment details (OS name and version, etc.)
- aws-record: 2.14.0 - Ruby: 3.4 - DynamoDB single-table data model - Falcon (fiber-based) server runtime
- Ngôn ngữ chính
- Ruby
- Star
- 318
- Fork
- 44
- Merge trung bình
- 2 ngày 21 giờ
- Pull request đã merge (30 ngày)
- 5
Chuẩn bị môi trường
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của aws/aws-record-ruby
-
feature-request
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 38/100
aws/aws-record-ruby#155 · 1 bình luận ·
-
feature-request p2 queued
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
aws/aws-record-ruby#153 · 2 bình luận ·
-
feature-request
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 35/100
aws/aws-record-ruby#152 · 3 bình luận ·
-
feature-request
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 35/100
aws/aws-record-ruby#147 · 1 bình luận ·
-
feature-request
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 35/100
aws/aws-record-ruby#142 · 6 bình luận ·
Tất cả issue của aws/aws-record-ruby
Issue tương tự
-
Độ khó 1/5 1-3 giờ Mức phù hợp với người mới 94/100
zerocracy/judges-action#2803 ·
Maintainer thường phản hồi trong vòng 5 ngày
-
Again Fix typo in comment in formatOnboardingAlertMessage [File :- course utils]Có thể đã có người làm @ThakuraAnki đã nhận hôm nay. Đang mởbug
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 95/100
WikiEducationFoundation/WikiEduDashboard#7158 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
enhancement
Độ khó 2/5 Nửa ngày Mức phù hợp với người mới 70/100
ericsaupe/scavenger-hunt#367 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Feature Feature Parity
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 82/100
valkey-io/valkey-glide-ruby#369 ·
Maintainer thường phản hồi trong vòng 2 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 85/100
Maintainer thường phản hồi trong vòng 1 ngày