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

[Bug]: Block applying in a spawned task synchronized incorrectly with in-memory update #344

Closed
polydez opened this issue May 4, 2024 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@polydez
Copy link
Contributor

polydez commented May 4, 2024

More context in this thread: #340 (comment)

From @bobbinth:

In general, this mechanism is used to support the following process:

  1. We start the database transaction and update the database. While this is going on, reads against the database are allowed as they read the previous state. So, the store can still serve all requests.
  2. Once the data has been written to the database, we lock the in-memory structures for update. At this point, the store blocks on all requests.
  3. We update in-memory structures. While this is going on, the store cannot serve any requests - so, it is critical to make this section as fast as possible.
  4. We commit the database transaction and release the lock on in-memory structures.

Because database transaction and in-memory structures live in different places, we need this channel - but I am wondering if there is some way we can refactor the structure to make the code simpler.

According to the code, current implementation has these bugs:

  1. When we lock in-memory data, we don't wait until commit is succeed and start to update in-memory. Even in a case of failed commit, in-memory data will be updated. By the way, we have a comment about this below.
  2. Once we finish update in-memory data, we unlock it instantly without waiting for finishing of database update.
  3. Error result of db.apply_block in tokio::spawn only logged, but not re-raised or handled.
@polydez polydez added the bug Something isn't working label May 4, 2024
@polydez polydez self-assigned this May 4, 2024
@bobbinth bobbinth added this to the v0.4 milestone May 4, 2024
@hackaugusto
Copy link
Contributor

Is that a problem though? The implementation is just ensuring that reads to the in-memory representation are consistent with the DB. If the commit fails, then the whole server should come down, is it only being logged?

@hackaugusto
Copy link
Contributor

Oh, okay. It seems there is a block store struct that also writes data to disk but from the in-memory side of things, so the existing code seems broken indeed.

@polydez polydez moved this to In Progress in User's testnet May 27, 2024
@polydez polydez moved this from In Progress to Done in User's testnet May 30, 2024
@polydez polydez moved this from Done to In Review in User's testnet May 30, 2024
@bobbinth
Copy link
Contributor

bobbinth commented Jun 3, 2024

Closed by #370.

@bobbinth bobbinth closed this as completed Jun 3, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in User's testnet Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants