[PUT IN RELEASE NOTES] Set a default value of RESTATE_ROCKSDB_TOTAL_MEMORY_SIZE in helm chart

[jackkleeman]

Currently the default resources in the helm chart are really small, and more importantly, they are out of line with the default memory size.

My suggestion is that we increase the resources to match the default memory size of 6G - ie, 8G memory req/limit, 1:4 cpu to memory request ratio so 2 cpu - and also start specifying the memory size more explicitly so users can see that they need to override it.

This will be a breaking change for users that are not specifying their resources, and needs to be noted in the release notes for that reason. It is breaking in the sense that if you don't have enough capacity for the node any more, the replaced pod will not schedule. However this would be noticed immediately. In other cases it will still schedule, and performance will be improved, so there is only a cost impact, which I think is ok (the user can always specify the resources they want if needed).

However, it is NOT a breaking change for those who are specifying their resources but aren't specifying the rocks limit, which i think is probably very common. in those cases, the rocksdb memory size will stay 6G, but we will have the release notes and a logline encouraging them to consider changing it if it doesn't match their overrided resources.

Alternatives considered;

• We could add the memory size env to match the current default resources, and not change those resources. This would potentially reduce the memory from 6G for any users that have not explicitly overriden this env, which will likely be a 'silent' performance degradation
• We could just document that these should be kept in line, and hope users notice this in release notes etc. The logline in Warn if cgroup memory limit is misaligned with rocksdb memory limit #3925 might help, but this still feels like quite the footgun.
• We could set some flag that instructs restate to automatically pick the memory limits to be 75% of the process limit. I'm inclined to avoid this kind of magic for the time being.

[github-actions]

Test Results

  7 files  ±0    7 suites  ±0   2m 34s ⏱️ +2s
 47 tests ±0   47 ✅ ±0  0 💤 ±0  0 ❌ ±0 
200 runs  ±0  200 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit f58b5d3. ± Comparison against base commit 5079b7e.

♻️ This comment has been updated with latest results.

[tillrohrmann]

Thanks for updating our helm charts with the new default memory limits @jackkleeman. The changes look good to me. +1 for including it in the release notes as this change can be problematic for some users. +1 for merging.

[tillrohrmann]

The one alternative I was thinking of is whether we could use K8s downward API to automatically derive the value based on the configured memory limit (specific to the helm chart). But it would also add some form of auto magic (which probably should only happen if the rocksdb total memory hasn't been set explicitly).

[jackkleeman]

not easily... we can indeed get the memory request or limit as an env var, but it wont necessarily be a byte count, it would be something like '2Gi' which needs parsing and them multiplying by 0.75 or whatever. imo if we do this we should do it inside Restate, with an option to set memory to some ratio of the process memory limit (which we try to get from the cgroup)

[jackkleeman]

but i am largely against this kind of magic, best to solve this with clear documentation. imo, a docs page like 'these are the 4 parameters you should pick when building a new cluster' would be really helpful