Skip to content

Conversation

@izabera
Copy link
Collaborator

@izabera izabera commented Nov 21, 2025

these fields are all atomic so the accesses are safe, but different threads end up stepping on each other's toes when accessing different fields

this adds padding and groups together the fields accessed at the same time

  • the first group is the hot path, which are updated together in the main event loop
  • then there's the request/response management group
  • then the reader group
  • then the catchup group
  • and finally there's a misc metadata group, which are touched independently at different times

@bitonic
Copy link
Collaborator

bitonic commented Nov 21, 2025

I personally wouldn't bother -- this is going to be totally immaterial performance-wise, and obviously prone to breaking if the code changes, and so it is at a very high risk of just remaining in the code as is without anybody caring about whether the groupings still make sense.

@mcrnic mcrnic added the ci If given to a PR, the PR will run CI checks. label Nov 24, 2025
Copy link
Collaborator

@mcrnic mcrnic left a comment

Choose a reason for hiding this comment

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

I don't mind but I personally would not bother. LogsDB is single threaded at the moment. The only other thing accessing these stats is stat exporter which kicks off once a minute.

@bitonic
Copy link
Collaborator

bitonic commented Nov 24, 2025

BTW, If we were to merge this, I'd definitely comment the alignas. As a compromise, I think merging without the alignas is harmless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci If given to a PR, the PR will run CI checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants