ref(project-cache): Split project cache into shared and private parts #4147
Conversation
Dav1dde
force-pushed
the
dav1d/remove-metric-meta
branch
from
acf88d4 to
d5012d9
Compare
Dav1dde
force-pushed
the
dav1d/sypc
branch
5 times, most recently
from
f52acfc to
7cba95b
Compare
| /// - `message`: The type of message that was processed. | ||
| ProjectCacheMessageDuration, | ||
| /// Timing in milliseconds for processing a message in the buffer service. | ||
| /// - `task`: The type of the task the project cache does. |
There was a problem hiding this comment.
The message is called ProjectCacheTaskDuration, it's a task, because the timer measures more than just the message, it also measures eviction (the entire tokio::select).
Also note this is a metric for the new service, the old one has new metrics.
| .send(ProjectEvent::Evicted(project_key)); | ||
| }; | ||
|
|
||
| self.store.evict_stale_projects(&self.config, on_evict); |
There was a problem hiding this comment.
You mentioned in the PR description that there is less garbage in this version, why is that? Isn't the amount of project configs that need deconstruction still the same?
There was a problem hiding this comment.
It does a slightly better handling of revisions which re-creates less projects. Where we before did always send back a project from the source, the source itself can now signal that there was no change. This is relevant for the sources where we peek into the deserialized contents and compare revisions (Redis).
|
|
||
| let _ = self | ||
| .project_events_tx | ||
| .send(ProjectEvent::Ready(project_key)); |
There was a problem hiding this comment.
Does this fail if the queue is full? If so, should we log an error just so we know we hit a "should not happen" situation?
There was a problem hiding this comment.
This also fails when there are no receivers, which may happen during shutdown or in testing.
There was a problem hiding this comment.
Can we differentiate these two cases? It would be good to notice somehow if the queue is full.
Makes the project cache concurrently accessible using a lockfree shared map.
no_cachequeries, they were not used for a long time and simplify the design (no need to make project accesses awaitable).The PR also removed all project cache handling from the old (now called legacy) project cache and moved it to a newer, simplified project cache which only does the book-keeping for the actual project cache. The old Spool V1 implementation and some message handling remains in the legacy project cache. The spool v1 will eventually be removed, the other messages can now be replaced and no longer need to be messages and can be removed in follow up PRs. Some messages likes
ProcessMetricshave already been removed.The garbage disposal has been removed, with the new organization we generate a lot less overall 'trash' which needs to be disposed as well as it is no longer necessary to offload the disposal to another thread, since the service itself has a lot less volume/work to do and service latency is no longer a major concern for stability.
It is now possible to subscribe to project cache events, this is done using a tokio broadcast channel, the channel is inherently size limited, which is problematic, since currently subscribers cannot deal with lag and cannot miss any messages. To prevent this (until we have a better way), the channel is set to a maximum size which should theoretically be impossible to reach. This is more described in code itself.
Functional changes: