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

track loaded sessions to enable concurrent access #37

Merged
merged 3 commits into from Oct 10, 2023

Conversation

maxcountryman
Copy link
Owner

This moves to a pattern of tracking loaded sessions via a hash set. By doing so, we can share sessions concurrently. In other words, we no longer need a lock to protect the session for the duration of the request and instead are able to share sessions between concurrent tasks.

Why do we need this at all? A data race can otherwise occur when a session has been loaded but its modifications have not been stored. For instance, another task may load the session from the store in the meantime and this load will overwrite pending modifications as it will effectively replace the session in memory.

One solution to this problem is to lock the request, as we've done in a stopgap capacity. This unscalable at present and even when refined, i.e. to lock per session as opposed for all sessions, limits concurrency over the session itself. In practice, sessions can only be operated on serially and are not available concurrently.

A more flexible approach is implemented here, allowing for concurrent memory access by limiting loads from the store to situations where no session exists in the loaded set. Put another way, sessions are tracked such that they can be used instead of loading from the store thus ensuring that pending modifications are not lost by a load.

See #36 for more details about the stopgap locking behavior; this replaces that approach altogether.

This moves to a pattern of tracking loaded sessions via a hash set. By
doing so, we can share sessions concurrently. In other words, we no
longer need a lock to protect the session for the duration of the
request and instead are able to share sessions between concurrent tasks.

Why do we need this at all? A data race can otherwise occur when a
session has been loaded but its modifications have not been stored. For
instance, another task may load the session from the store in the
meantime and this load will overwrite pending modifications as it will
effectively replace the session in memory.

One solution to this problem is to lock the request, as we've done in a
stopgap capacity. This unscalable at present and even when refined, i.e.
to lock per session as opposed for all sessions, limits concurrency over
the session itself. In practice, sessions can only be operated on
serially and are not available concurrently.

A more flexible approach is implemented here, allowing for concurrent
memory access by limiting loads from the store to situations where no
session exists in the loaded set. Put another way, sessions are tracked
such that they can be used instead of loading from the store thus
ensuring that pending modifications are not lost by a load.

See #36 for more details about the stopgap locking behavior; this
replaces that approach altogether.
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #37 (965a861) into main (1cafc8a) will increase coverage by 1.43%.
The diff coverage is 81.48%.

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   77.77%   79.21%   +1.43%     
==========================================
  Files          13       13              
  Lines         423      433      +10     
==========================================
+ Hits          329      343      +14     
+ Misses         94       90       -4     
Files Coverage Δ
src/lib.rs 33.33% <ø> (+33.33%) ⬆️
src/mongodb_store.rs 96.66% <ø> (+3.11%) ⬆️
src/sqlx_store/mysql_store.rs 97.29% <ø> (+2.56%) ⬆️
src/sqlx_store/postgres_store.rs 87.80% <ø> (+2.09%) ⬆️
src/sqlx_store/sqlite_store.rs 86.11% <ø> (+2.32%) ⬆️
src/session.rs 73.07% <75.00%> (-0.40%) ⬇️
src/service.rs 72.83% <84.21%> (+0.23%) ⬆️

@maxcountryman maxcountryman merged commit 6dab34b into main Oct 10, 2023
13 checks passed
@maxcountryman maxcountryman deleted the session-load-save-data-race branch October 10, 2023 21:32
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.

None yet

1 participant