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

Atomic counters in statistics #1504

Open
fgalan opened this issue Nov 13, 2015 · 1 comment
Open

Atomic counters in statistics #1504

fgalan opened this issue Nov 13, 2015 · 1 comment
Labels

Comments

@fgalan
Copy link
Member

fgalan commented Nov 13, 2015

Staticist operation includes many counters, as for instance the counter for request number per type (included since the very first version of the statistics functionality) or the timing counters and notification queue counters recently introduced.

These counters are potencially incremented from concurrent threads but currently we are not using any mechanism in our code to ensure that increments are done in atomic way. Thus, considering counter C with value N, it may happend that if thread A want to increase in one and thread B wants to increse in one, the counter ends with N+1 at the end (instead of N+2).

Some parts (the ones related with notification queue) are using a mechanism based in __sync_fetch_and_add which could be extended to all the other counters. Another alternative is to use boost::atomic<D> but unfortunatelly it is not include in the Boost library version used by Cb.

There could be a trade off between precision and performance if the coded implementation for these builtins uses internal exclusion mechanisms which make some threads to wait while the other modifies the counter. However, as long as we can control statistics generation with CLI parameters (probably CLI switch for statistics should be also reviewed when addressing this issue), it shouldn't be a problem given that:

  • If CB is going to be used in high-load environments, then the CLI parameter will be used to disable statistics at get maximum performance.
  • If the CB is going to be used in not hihg-load environments, then performance is not critical.

(Initally assinged to 0.26.0, but maybe it would be deferred to a future milestone after internal discussion).

@fgalan fgalan added the backlog label Nov 13, 2015
@fgalan fgalan added this to the 0.26.0 milestone Nov 13, 2015
@fgalan fgalan modified the milestones: 0.26.0, 0.27.0, 0.28.0 Nov 30, 2015
@fgalan fgalan modified the milestones: 0.28.0, 0.29.0 Feb 4, 2016
@fgalan fgalan modified the milestones: 1.0.0, 1.1.0, 1.2.0 Mar 17, 2016
@fgalan fgalan modified the milestones: 1.2.0, 1.3.0 May 3, 2016
@fgalan fgalan modified the milestones: 1.3.0, 1.4.0 May 31, 2016
@fgalan fgalan modified the milestones: 1.4.0, 1.5.0 Sep 6, 2016
@fgalan fgalan modified the milestones: 1.5.0, 1.6.0 Oct 28, 2016
@fgalan fgalan modified the milestones: 1.6.0, 1.7.0 Nov 28, 2016
@fgalan
Copy link
Member Author

fgalan commented Dec 2, 2016

The API metrics (#2750) should (and will :) use a mechanism to ensure atomicity.

Regarding the other "debug level" statistics at /statistics, this issue remains opened (although probably is less important each passing day).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant