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.

Sign up for GitHub

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

Jump to bottom

Fix bug in object_by_id_cache #20450

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

Fix bug in object_by_id_cache #20450

wants to merge 2 commits into from
+215 −29

Commits on Nov 27, 2024

  1. Fix bug in object_by_id_cache. 

    Suppose that a reader thread is trying to cache an object that it just
read, while a writer thread is trying to cache an object that it just
wrote. The writer thread definitionally has the latest version. The
reader thread may be out of date. While we previously took some care
to not replace a new version with an old version, this did not take into
account evictions, and so the following bug was possible:

      READER                            WRITER
      read object_by_id_cache (miss)
      read dirty set (miss)
                                        write to dirty
      read db (old version)
                                        write to cache (while holding dirty lock)
                                        cache entry is evicted
      write to cache

There is no way for the reader to tell that the value is is caching is
out of date, because the update to date entry is already gone from the
cache.

We fix this by requiring reader threads to obtain a ticket before they
read from the dirty set and/or db. Tickets are expired by writers. Then,
the above case looks like this:

      READER                            WRITER
      get ticket
      read cache (miss)
      read dirty (miss)
                                        write dirty
      read db (old version)
                                        expire ticket
                                        write cache (while holding dirty lock)
                                        cache eviction
      no write to cache (ticket expired)

Any interleaving of the above either results in the reader seeing a
recent version, or else having an expired ticket.
    @mystenmark
    mystenmark committed Nov 27, 2024
    Configuration menu
    Copy the full SHA
    98b7edd View commit details
    Browse the repository at this point in the history

  2. lint

    @mystenmark
    mystenmark committed Nov 27, 2024
    Configuration menu
    Copy the full SHA
    28244db View commit details
    Browse the repository at this point in the history