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

perf: Try to load previous seqNr if missing, in validation #1197

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

patriknw
Copy link
Member

  • makes it more safe to evict from memory, but keep more in database
  • a step towards lazy loading of offsets, but full lazy loading would require many more changes due to the concurrency model used in the R2dbcOffsetStore
  • otherwise it will use timestampOf, which are be more costly for gRPC projections
  • it still falls back to timestampOf as last resort

@patriknw patriknw requested a review from pvlugter September 12, 2024 13:19
SELECT seq_nr, timestamp_offset
FROM $timestampOffsetTable WHERE slice = ? AND projection_name = ? AND persistence_id = ?
ORDER BY seq_nr DESC
LIMIT 1"""
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried about the index for this since timestamp_offset is before persistence_id in:

PRIMARY KEY(slice, projection_name, timestamp_offset, persistence_id, seq_nr)

but might be fine?

 Limit  (cost=4.31..4.32 rows=1 width=16) (actual time=0.137..0.138 rows=1 loops=1)
   ->  Sort  (cost=4.31..4.32 rows=1 width=16) (actual time=0.135..0.136 rows=1 loops=1)
         Sort Key: seq_nr DESC
         Sort Method: quicksort  Memory: 25kB
         ->  Index Only Scan using akka_projection_timestamp_offset_store_pkey on akka_projection_timestamp_offset_store  (cost=0.28..4.30 rows=1 width=16) (actual time=0.085..0.087 rows=2 loops=1)
               Index Cond: ((slice = 449) AND (projection_name = 'b267e71d-7ed9-4f02-8726-b96b0783ae7a'::text) AND (persistence_id = 'p1'::text))
               Heap Fetches: 0

The reason timestamp_offset is before is for the deletes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also have an optional extra index for this.

@patriknw
Copy link
Member Author

One thing that this could enable as a next step would be to decouple the db deletes from how much it holds in memory. I'd like to keep things in db for much longer than the in-memory window.

Copy link
Contributor

@pvlugter pvlugter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@patriknw patriknw force-pushed the wip-validate-load-patriknw branch from 6b5d439 to 123524b Compare October 9, 2024 14:52
* makes it more safe to evict from memory, but keep more in database
* a step towards lazy loading of offsets,
  but full lazy loading would require many more changes due to the concurrency model used in the R2dbcOffsetStore
* otherwise it will use timestampOf, which are be more costly for gRPC projections
* it still falls back to timestampOf as last resort
@patriknw patriknw force-pushed the wip-validate-load-patriknw branch from 123524b to d825c85 Compare October 18, 2024 09:41
@patriknw patriknw merged commit e956248 into main Oct 18, 2024
20 of 21 checks passed
@patriknw patriknw deleted the wip-validate-load-patriknw branch October 18, 2024 10:32
patriknw added a commit that referenced this pull request Oct 18, 2024
* perf: Try to load previous seqNr if missing, in validation

* makes it more safe to evict from memory, but keep more in database
* a step towards lazy loading of offsets,
  but full lazy loading would require many more changes due to the concurrency model used in the R2dbcOffsetStore
* otherwise it will use timestampOf, which are be more costly for gRPC projections
* it still falls back to timestampOf as last resort

* different sql for SqlServer

(cherry picked from commit e956248)
patriknw added a commit that referenced this pull request Oct 18, 2024
…1229)

* perf: Try to load previous seqNr if missing, in validation

* makes it more safe to evict from memory, but keep more in database
* a step towards lazy loading of offsets,
  but full lazy loading would require many more changes due to the concurrency model used in the R2dbcOffsetStore
* otherwise it will use timestampOf, which are be more costly for gRPC projections
* it still falls back to timestampOf as last resort

* different sql for SqlServer

(cherry picked from commit e956248)
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.

3 participants