kvs: support configuration of max operations count

[chu11]

Per discussion in #6125, denial-of-service attacks could be made against the KVS by very very large KVS transactions.

Support two configurations for capping the number of transactions made by users. One for each individual transaction made by a caller and one for the combined total of operations from a fence.

For the time being, I made the default 64K for the transaction cap and 1M for the fence cap.

I made this WIP only b/c those defaults may be tweaked depending on what stats we get from the prior PR #6556. I would like to merge only after we gather a bit of data, although I'd be quite shocked if we have to adjust the defaults. Edit: Or alternately, if we'd like to just get the code in, we could default the max to LLONG_MAX and lower the default at a later time.

Only other thought is I decided to return the errno E2BIG if we went across a max cap boundary. It's possible there is a superior errno for this, I picked it b/c I thought "ehhh that's not bad".

[chu11]

re-pushed, removing all fence related parts of this PR, since KVS fence support is now gone

[chu11]

re-pushed, removing all the fence stuff that is no longer relevant b/c KVS fence support was removed

[chu11]

Some small set of transaction stats

 "transaction-opcount": {
  "commit": {
   "count": 48829,
   "min": 1,
   "mean": 1.976673698007309,
   "stddev": 6.318251377545355,
   "max": 398
  }
 },

the max number of ops in a transaction is 398, which means my default cap of 64K seems more than fine. I'll still monitor stats until release time, but removing WIP for now so this can be reviewed

[chu11]

saw this today

 "transaction-opcount": {
  "commit": {
   "count": 95589,
   "min": 1,
   "mean": 3.4195984893659341,
   "stddev": 42.249674028063289,
   "max": 11395
  }
 },

max of 11K ... perhaps upping to 128K over 64K would still be good but a bit safer.

[garlick]

Before we do this, we should probably investigate why we are seeing these high op-count commits.

I think stdout events are batched by this code, which triggers the commit of a batch of events based on a timer. Maybe it should also have a high water mark on the operation count and/or the cumulative size of the events.

https://github.com/flux-framework/flux-core/blob/master/src/common/libeventlog/eventlogger.c

If we can make sure flux-core code is well behaved, then it makes sense to me to impose a limit to avoid regressions and bad behavior by framework projects, but I should think we could set it much lower than 128K. We should also provide some way for API users to know what the limit is. For example, then the above code could set its high water mark accordingly. Maye we could even just make it a constant in kvs.h.

Note this PR in the title and a few other places uses "transaction" where "operation" is meant. IMHO the title should be "kvs: limit the number of operations per commit".

[chu11]

If we can make sure flux-core code is well behaved, then it makes sense to me to impose a limit to avoid regressions and bad behavior by framework projects, but I should think we could set it much lower than 128K. We should also provide some way for API users to know what the limit is. For example, then the above code could set its high water mark accordingly. Maye we could even just make it a constant in kvs.h.

Good point. I'll write up an issue to investigate this issue as well. However, this PR was initially developed under the idea to defend against a denial of service, so I think it is worthwhile to have the max independent of it. At this moment, a rogue user (or misbehaving code) could create a KVS transaction with like a bajillion operations in it right now.

[chu11]

with #7291 being merged, came by to update this old PR. The default cap is now 16384, matching the default cap in the libeventlog eventlogger library.

As a side note, I looked around and the largest "max ops" I saw in the wild was 29K on tuolomne. The presumption is it is from the eventlogger which will handle it. Before, the max I saw was 11K. We could bump to 32K if we want to be super safe.

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.56%. Comparing base (a2a4a7a) to head (bfb5d59).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/modules/kvs/kvs.c	90.90%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6581      +/-   ##
==========================================
- Coverage   83.56%   83.56%   -0.01%     
==========================================
  Files         562      562              
  Lines       93616    93638      +22     
==========================================
+ Hits        78231    78244      +13     
- Misses      15385    15394       +9     

Files with missing lines	Coverage Δ
src/modules/kvs/kvs.c	72.74% <90.90%> (+0.29%)	⬆️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.