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

Sync with upstream #6

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Sync with upstream #6

wants to merge 3 commits into from

Conversation

agodnic
Copy link

@agodnic agodnic commented Feb 27, 2025

Summary

Sync with the latest upstream commit. In particular, add support for the online-only parameter in GET /v2/accounts to search for online accounts.

Note that the parameter is disabled by default.

Performance

The performance of is-online=true lookups can be improved with the following index:

-- create the index
ledgerdb=# CREATE INDEX account_idx_online_only
        ON account ((((account_data ->> 'onl')) = '1'), addr)
        WHERE (NOT deleted) AND ((account_data ->> 'onl') IS NOT NULL) AND ((account_data ->> 'voteLst') IS NOT NULL)
;
CREATE INDEX

-- Verify the index is being used by the query optimizer.
-- This query corresponds to /v2/accounts?limit=10&online-only=true&exclude=all
ledgerdb=# explain (analyze) WITH qaccounts AS (SELECT a.addr, a.microalgos, a.rewards_total, a.created_at, a.closed_at, a.deleted, a.rewardsbase, a.keytype, a.account_data FROM account a WHERE NOT a.deleted AND (a.account_data->>'onl' IS NOT NULL) and (account_data->>'voteLst' IS NOT NULL) and ((a.account_data->>'onl') = '1') ORDER BY a.addr ASC LIMIT 10) SELECT za.addr, za.microalgos, za.rewards_total, za.created_at, za.closed_at, za.deleted, za.rewardsbase, za.keytype, za.account_data FROM qaccounts za ORDER BY za.addr ASC;
                                                                      QUERY PLAN                                                                      
------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.28..37.70 rows=10 width=103) (actual time=0.023..0.038 rows=10 loops=1)
   ->  Index Scan using account_idx_online_only on account a  (cost=0.28..407632.13 rows=108930 width=103) (actual time=0.023..0.036 rows=10 loops=1)
         Index Cond: (((account_data ->> 'onl'::text) = '1'::text) = true)
 Planning Time: 0.268 ms
 Execution Time: 0.049 ms
(5 rows)

gmalouf and others added 3 commits January 13, 2025 16:04
* Revert "Disable rewinding and reject the  query param on account lookups and searches. (algorand#1630)"

This reverts commit 249016c. The synthetic transaction implementation of payouts in the indexer should allow balances retrieved via rewind to calculate as before.

* GCI lint warning fix.

* Add HeartbeatTxn to be ignored during rewinding.
* Search for online accounts in `GET /v2/accounts`

* Fix typo

* Fix an issue in the `online-only` filter

Fix an issue in the underlying SQL query for the `online-only`
parameter.

The previous SQL query was returning online accounts with null key
material. This commit filters out those accounts - only accounts
with non-null key material are returned.

* Style fixes

* Add e2e test coverage for `online-only` parameter

* Disable `online-only` parameter by default.

In the endpoint `GET /v2/accounts`, disable the `online-only` parameter
by default.

* Remove unnecesary code

* Increase test coverage for `online-only` param

* Style fixes

* Update disabled parameter list in README.md

* Update param list in `DisablingParametersGuide.md`

* Remove duplicated code

* Add extra parenthesis in SQL expression

* Add missing table alias in SQL query

* Improve readability in e2e test

* Remove unnecessary comments

* Add comments to e2e test

* Update misleading comment
@agodnic agodnic marked this pull request as ready for review February 27, 2025 00:11
@agodnic agodnic requested a review from urtho February 27, 2025 00:11
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.

2 participants