Make commandlog-request-larger-than and commandlog-reply-larger-than become SIGNED_MEMORY_CONFIG

[enjoy-binbin]

After #3443, we have a new SIGNED_MEMORY_CONFIG, which enables the memory
configuration to support storing the value -1.

Make commandlog-request-larger-than and commandlog-reply-larger-than become
SIGNED_MEMORY_CONFIG, so we can use it in a memory way.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.55%. Comparing base (be52d8b) to head (8e802cf).

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2648      +/-   ##
============================================
+ Coverage     76.44%   76.55%   +0.10%     
============================================
  Files           157      157              
  Lines         79035    79035              
============================================
+ Hits          60421    60504      +83     
+ Misses        18614    18531      -83     

Files with missing lines	Coverage Δ
src/config.c	78.33% <ø> (ø)

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[soloestoy]

at first I also wanted to make it as memory config, but memory config cannot be negative, so we cannot use memory config

[enjoy-binbin]

at first I also wanted to make it as memory config, but memory config cannot be negative, so we cannot use memory config

right, i forgot memory config can not use the negative number, but the code does not have any practical limitations? Like we can add a new xxx_CONFIG and make it support -1? It's a bit inconvenient not to be able to just use memory config for this memory bytes number.

[madolson]

Wouldn't it be possible to make it so that memory configs also support negative numbers? It's stored in an signed long long, so that should be be possible.

Changed my mind

[madolson]

Ok, it seems like folks in the monday meeting we were aligned about moving it to memory, but there some concerns about the change from ull2string -> ll2string which might truncate some values.

[enjoy-binbin]

Ok, it seems like folks in the monday meeting we were aligned about moving it to memory, but there some concerns about the change from ull2string -> ll2string which might truncate some values.

yes, that is a problem...

And we actually have the problem in the unstable branch, like if we set maxmemory to 9223372036854775808 and doing a config rewrite, we will end up a maxmemory -8589934592gb in the config file. This will be a bug, i will try to fix it first in a different PR.

/* Rewrite a simple "option-name <bytes>" configuration option. */
void rewriteConfigBytesOption(struct rewriteConfigState *state,
                              const char *option,
                              long long value,
                              long long defvalue) {
    char buf[64];
    int force = value != defvalue;
    sds line;

    rewriteConfigFormatMemory(buf, sizeof(buf), value);
    line = sdscatprintf(sdsempty(), "%s %s", option, buf);
    rewriteConfigRewriteLine(state, option, line, force);
}

[zuiderkwast]

LGTM.

It's completely backward compatible, right?

[enjoy-binbin]

It's completely backward compatible, right?

Yes, i think so, i tested these locally, and everything is working normally

• commandlog-request-larger-than 9223372036854775807
• commandlog-request-larger-than -1
• commandlog-request-larger-than 0

[zuiderkwast]

@enjoy-binbin The faster-failover test failed in the Codecov test run.

The replica is the best ranked replica in the shard, but there is another primary failure and primary rank is 1.

* Start of election delayed for 395 milliseconds (rank #0, primary rank #1, offset 347).

https://github.com/valkey-io/valkey/actions/runs/24223553275/job/70719922425?pr=2648#step:6:6469

This makes the test case flaky. Can we fix it in some way? For example we can restart the test case if this happens?

[enjoy-binbin]

This makes the test case flaky. Can we fix it in some way? For example we can restart the test case if this happens?

Yes, i think we can give a more chances, like we did in other tests. Let me try it.