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

indexer stability: revert diesel-async changes #11017

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

gegaowp
Copy link
Contributor

@gegaowp gegaowp commented Apr 18, 2023

Description

The reason is that read_only queries will still build tx, see code pointer and when Diesel gets concurrent tx, it will check if some other tx is in process here, if there is another on-going tx, even it is a read-only tx, Diesel will return error AlreadyInTransaction

More context can be found in #sui_testnet_n_inc_307

Test Plan

CI

@vercel
Copy link

vercel bot commented Apr 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Apr 18, 2023 3:07pm
explorer-storybook ⬜️ Ignored (Inspect) Apr 18, 2023 3:07pm
sui-wallet-kit ⬜️ Ignored (Inspect) Apr 18, 2023 3:07pm
wallet-adapter ⬜️ Ignored (Inspect) Apr 18, 2023 3:07pm

@gegaowp gegaowp changed the title indexer: read_only to read_only_blocker indexer: read_only to read_only_blocking Apr 18, 2023
@gegaowp gegaowp marked this pull request as draft April 18, 2023 00:27
@gegaowp
Copy link
Contributor Author

gegaowp commented Apr 18, 2023

it needs a bit more work to revert this back to blocking, working on it now

@gegaowp gegaowp force-pushed the read-only-blocking branch 2 times, most recently from 6a64c75 to 2571bae Compare April 18, 2023 02:36
@gegaowp gegaowp changed the title indexer: read_only to read_only_blocking indexer: revert diesel-async changes Apr 18, 2023
@gegaowp gegaowp force-pushed the read-only-blocking branch from 9b7f7fc to 94a21cc Compare April 18, 2023 14:16
@gegaowp gegaowp force-pushed the read-only-blocking branch from 94a21cc to dd99c0e Compare April 18, 2023 14:55
@gegaowp gegaowp marked this pull request as ready for review April 18, 2023 15:48
@gegaowp gegaowp changed the title indexer: revert diesel-async changes indexer stability: revert diesel-async changes Apr 18, 2023
@gegaowp gegaowp merged commit bd2b509 into MystenLabs:main Apr 18, 2023
@gegaowp gegaowp deleted the read-only-blocking branch April 18, 2023 20:36
gegaowp added a commit to gegaowp/sui that referenced this pull request Apr 18, 2023
## Description 

The reason is that read_only queries will still build tx, see code
[pointer](https://github.com/MystenLabs/sui/blob/main/crates/sui-indexer/src/store/mod.rs#L24-L34)
and when Diesel gets concurrent tx, it will check if some other tx is in
process
[here](https://github.com/diesel-rs/diesel/blob/4b2ebca2ddb25f08bf9107869f064f5774efbb83/diesel/src/connection/transaction_manager.rs#L286),
if there is another on-going tx, even it is a read-only tx, Diesel will
return error AlreadyInTransaction

More context can be found in #sui_testnet_n_inc_307

## Test Plan 

CI
gegaowp added a commit that referenced this pull request Apr 18, 2023
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