Fix slot-migration-max-failover-repl-bytes unable to accept -1

[enjoy-binbin]

In valkey.conf, slot-migration-max-failover-repl-bytes allows setting
to -1 to disable the limit.

Setting this to -1 will disable this limit

But slot-migration-max-failover-repl-bytes is defined as MEMORY_CONFIG
and memtoull() rejects negative inputs, making it impossible to set the
value to -1 via config file or CONFIG SET.

>>> 'slot-migration-max-failover-repl-bytes "-1"'
argument must be a memory value

Introduce SIGNED_MEMORY_CONFIG flag for memory configs that also accept
plain negative number. When memtoull() fails and this flag is set, fall
back to string2ll() for parsing. Use ll2string() for CONFIG GET and
rewriteConfigNumericalOption() for CONFIG REWRITE when the value is
negative.

Add a serverAssert in initConfigValues() to enforce that PERCENT_CONFIG
and SIGNED_MEMORY_CONFIG are never combined on the same config, since
both use negative values with different semantics.

This means we have had this issue since it was introduced in #1949.

[murphyjacob4]

Makes sense! Thanks for the fix

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.52%. Comparing base (e8ca6c0) to head (442125d).
⚠️ Report is 8 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3443      +/-   ##
============================================
- Coverage     76.52%   76.52%   -0.01%     
============================================
  Files           157      157              
  Lines         79025    79035      +10     
============================================
+ Hits          60477    60479       +2     
- Misses        18548    18556       +8     

Files with missing lines	Coverage Δ
src/cluster_migrateslots.c	91.87% <ø> (-0.39%)	⬇️
src/config.c	78.02% <100.00%> (+0.31%)	⬆️
src/server.h	100.00% <ø> (ø)

... and 25 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.

[zuiderkwast]

Great!

So was slot-migration-max-failover-repl-bytes -1 not tested before?

[enjoy-binbin]

So was slot-migration-max-failover-repl-bytes -1 not tested before?

Yes, i added a test, this require a log change.

[zuiderkwast]

The failing test case "Main db not affected when fail to diskless load" has this pattern of writing many commands in a pipeline and reading the replies only afterwards:

    set num 10000
    set value [string repeat A 1024]
    set rd [valkey_deferring_client valkey $master_id]
    for {set j 0} {$j < $num} {incr j} {
        $rd set $j $value
    }
    for {set j 0} {$j < $num} {incr j} {
        $rd read
    }

I have fixed similar tests before using CLIENT REPLY OFF (#3430, #2483). I think we need the same fix here.

[EDIT] Fix is here: #3452

[zuiderkwast]

Looks good. I found some minor ideas but nothing blocking.

I didn't review the tests very much.

[sarthakaggarwal97]

Nice fix!