BuildableSearch#complete!.to_a auto-paginates silently and Limit is per-page (companion to #153)
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 25/100
- Tipo di issue
- Funzionalità
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Tranquilla
- Stack tecnologico
- aws, ruby
- Ambito
- backend-api-design, databases
Direzione di ricerca
Inizia leggendo Aws::Record::BuildableSearch#complete! e Aws::Record::ItemCollection#to_a e #each, poi esamina le spec esistenti sul comportamento di paginazione e iterazione. Il lavoro comprende la documentazione, metodi terminali espliciti e una modalità di iterazione sicura; è completato quando i contratti richiesti, il comportamento in caso di errore e i limiti di paginazione sono implementati e coperti dai test.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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
- Lingua principale
- Ruby
- Stelle
- 318
- Fork
- 44
- Merge medio
- 3g 15h
- PR unite (30g)
- 4
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Ha un modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di aws/aws-record-ruby
-
feature-request
Difficoltà 5/5 Più di una settimana Idoneità per principianti 38/100
aws/aws-record-ruby#155 · 1 commento ·
-
feature-request p2 queued
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
aws/aws-record-ruby#153 · 2 commenti ·
-
feature-request
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
aws/aws-record-ruby#152 · 3 commenti ·
-
feature-request
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
aws/aws-record-ruby#147 · 1 commento ·
-
feature-request
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
aws/aws-record-ruby#142 · 6 commenti ·
Tutte le issue di aws/aws-record-ruby
Issue simili
-
Local evaluation buckets percentage splits with the server key, so results differ from FlagsmithAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
-
Add PrestashopApertarequest
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
endoflife-date/endoflife.date#11303 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 66/100
FreeTubeApp/FreeTube-Docs#245 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
I maintainer di solito rispondono entro 1 giorno