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

exp/lighthorizon: Correctly paginate responses to respect query limits #4430

Closed
Tracked by #4571
sreuland opened this issue Jun 10, 2022 · 0 comments · Fixed by #4431
Closed
Tracked by #4571

exp/lighthorizon: Correctly paginate responses to respect query limits #4430

sreuland opened this issue Jun 10, 2022 · 0 comments · Fixed by #4431
Assignees

Comments

@sreuland
Copy link
Contributor

sreuland commented Jun 10, 2022

What problem does your feature solve?

page limit parameter on requests should determine the response results sizee
contributes to #4317 development tasks, specifically to implement correct paging behavior for limit

What would you like to see?

A request to serve ?limit=N of a resource should actually serve N of that resource, rather than reading N ledgers and serving 0 <= M <= N instances matching the requested resource (start in ./archive/main.go).


Details

In ./archive/wrapper.go, we see two methods: GetTransactions() and GetOperations(). For both of these cases, the limit parameter is reached via the following (abridged) code:

txns = append(txns, common.Transaction{TransactionEnvelope: &tx.Envelope, /* etc. */})
if int64(len(txns)) == limit {
	return txns, nil
}

where tx comes from reader.Read(), an ingest.LedgerTransactionReader instance built from ledger. ledger comes from Wrapper.GetLedger(ledgerSequence) (where the sequence is derived from the cursor), and this corresponds to a lookup to a txmeta dump (see ./main.go).

Key point: We want to see indices utilized when finding ledgers to look up, rather than just using them as a starting point for the cursor (see actions/transaction.go: all the index store is used for is advancing cursor to the next active ledger). We should move index usage to within the GetTransactions() method and advance to the next active ledger, pulling transactions until the limit is fulfilled.

@sreuland sreuland self-assigned this Jun 10, 2022
@sreuland sreuland mentioned this issue Jun 10, 2022
7 tasks
@Shaptic Shaptic changed the title page limit parameter on requests should determine the response results size exp/lighthorizon: Correctly paginate responses to respect query limits Jun 10, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 13, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 13, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 14, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 20, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 21, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 23, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 23, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 23, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 23, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 24, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 29, 2022
sreuland added a commit to sreuland/go that referenced this issue Jun 29, 2022
@Shaptic Shaptic closed this as completed Jul 15, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
sreuland added a commit to sreuland/go that referenced this issue Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants