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

Use COPY to speed up ClaimableBalancesChangeProcessor #5086

Closed
tamirms opened this issue Oct 19, 2023 · 3 comments
Closed

Use COPY to speed up ClaimableBalancesChangeProcessor #5086

tamirms opened this issue Oct 19, 2023 · 3 comments
Assignees
Labels
performance issues aimed at improving performance

Comments

@tamirms
Copy link
Contributor

tamirms commented Oct 19, 2023

In #4908 we observed that inserting rows is much faster using the postgres COPY command. We can use this property to improve the performance of the ClaimableBalancesChangeProcessor . The ClaimableBalancesChangeProcessor upserts rows into the claimable_balances and claimable_balance_claimants tables.

However, because claimable balances are either created or removed it turns out that we never have to modify existing rows of claimable_balances and claimable_balance_claimants. The create claimable balance operation creates a claimable balance and the claim operation removes the claimable balance from the ledger. There is no way to mutate a claimable balance once created.

This means we can update the ClaimableBalancesChangeProcessor to use COPY to insert into the the claimable_balances and claimable_balance_claimants tables. We expect that to be faster than the current code which bulk inserts rows into the claimable_balances and claimable_balance_claimants tables.

@tamirms
Copy link
Contributor Author

tamirms commented Oct 19, 2023

We can also use COPY to speed up inserts in the SignersProcessor

@mollykarcher mollykarcher added the performance issues aimed at improving performance label Oct 19, 2023
@mollykarcher mollykarcher moved this from Backlog to Current Sprint in Platform Scrum Oct 19, 2023
@urvisavla urvisavla mentioned this issue Oct 20, 2023
7 tasks
@urvisavla
Copy link
Contributor

When I switched the claimable balance processor to use FastBatchInsertBuilder,' which uses ‘COPY' instead of 'INSERT,' it worked in my local environment but it failed on the CI pipeline with the following error in json parsing pq: invalid input syntax for type json.

After investigating, I found that the pq library is encoding the json into hex (https://github.com/lib/pq/blob/master/encode.go#L590-L596). So for example, the claimants json [{"destination":"GC3C4AKRBQLHOJ45U4XG35ESVWRDECWO5XLDGYADO6DPR3L7KIDVUMML","predicate":{"unconditional":true}}] is encoded into this hex: \x5b7b2264657374696e6174696f6e223a224743334334414b5242514c484F4A34355534584733354553565752444543574O35584C44475941444O36445052334C374K494456554D4M222C227072656469636174E223a7b22756e636f6e646974696f6e616c223a747275657d7d5d. The error occurs because the backslash in \x is escaped as \\x (in the same library here https://github.com/lib/pq/blob/master/encode.go#L185-L186) making it invalid and so it fails on writing to the database.

According to the code this issue should occur for Postgres version 9.0 and above and the Postgres version I have locally is 15.4, so technically should have occurred locally. Surprisingly, it worked in the locally because of another bug in the pq library in getting the Postgres server version which caused this check to consistently fail when running locally, preventing the data from being encoded into hex. This also explained the discrepancy in CI failures Postgres versions 9.6.5 (failed) and 10.0 (passed).

Although this bug had been fixed for a while, we were still using the older version until a couple of days ago when we updated to the latest version from pq v1.2 to pq v1.10. Since the update, the issue happens both locally and in CI, as expected.

We had not seen this issue previously because we don’t use the 'COPY' method elsewhere.

@urvisavla
Copy link
Contributor

The pq repo has an open issue for the same error when using COPY with jsonb.

@urvisavla urvisavla moved this from Current Sprint to In Progress in Platform Scrum Oct 31, 2023
@tamirms tamirms changed the title Use COPY to insert into speed up ClaimableBalancesChangeProcessor Use COPY to speed up ClaimableBalancesChangeProcessor Nov 1, 2023
@urvisavla urvisavla moved this from In Progress to Needs Review in Platform Scrum Nov 7, 2023
@urvisavla urvisavla moved this from Needs Review to Done in Platform Scrum Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance issues aimed at improving performance
Projects
Status: Done
Development

No branches or pull requests

3 participants