From 349f01e86a3162614bbd17ade128fc96ee8cd857 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Fri, 28 Apr 2023 15:33:17 -0600 Subject: [PATCH] Change the absence of compression setting to default to "accept" In that mode, a server accepts and will switch to same compression level than the remote (if one is set) but will not initiate compression. So if all servers in a cluster do not have compression setting set, it defaults to "accept" which means that compression is "off". Signed-off-by: Ivan Kozlovic --- server/opts.go | 9 +++---- server/reload_test.go | 55 ++++++++++++++++++++----------------------- server/routes_test.go | 32 ++++++++++++------------- server/server.go | 4 ++-- 4 files changed, 48 insertions(+), 52 deletions(-) diff --git a/server/opts.go b/server/opts.go index bb4c1026..02656d7c 100644 --- a/server/opts.go +++ b/server/opts.go @@ -1669,7 +1669,7 @@ func parseCompression(c *CompressionOpts, tk token, mk string, mv interface{}) ( c.Mode = mv case bool: if mv { - c.Mode = CompressionS2Auto + c.Mode = CompressionS2Fast } else { c.Mode = CompressionOff } @@ -4786,13 +4786,14 @@ func setBaselineOptions(opts *Options) { } } } - // Default to compression "auto", unless we are running tests that specify - // what the default compression mode should be. + // Default to compression "accept", which means that compression is not + // initiated, but if the remote selects compression, this server will + // use the same. if c := &opts.Cluster.Compression; c.Mode == _EMPTY_ { if testDefaultClusterCompression != _EMPTY_ { c.Mode = testDefaultClusterCompression } else { - c.Mode = CompressionS2Auto + c.Mode = CompressionAccept } } } diff --git a/server/reload_test.go b/server/reload_test.go index 55c925d8..0cf39918 100644 --- a/server/reload_test.go +++ b/server/reload_test.go @@ -146,10 +146,9 @@ func TestConfigReloadUnsupported(t *testing.T) { MaxPingsOut: 2, WriteDeadline: 10 * time.Second, Cluster: ClusterOpts{ - Name: "abc", - Host: "127.0.0.1", - Port: -1, - Compression: CompressionOpts{Mode: CompressionS2Auto, RTTThresholds: defaultCompressionS2AutoRTTThresholds}, + Name: "abc", + Host: "127.0.0.1", + Port: -1, }, NoSigs: true, } @@ -219,10 +218,9 @@ func TestConfigReloadInvalidConfig(t *testing.T) { MaxPingsOut: 2, WriteDeadline: 10 * time.Second, Cluster: ClusterOpts{ - Name: "abc", - Host: "127.0.0.1", - Port: -1, - Compression: CompressionOpts{Mode: CompressionS2Auto, RTTThresholds: defaultCompressionS2AutoRTTThresholds}, + Name: "abc", + Host: "127.0.0.1", + Port: -1, }, NoSigs: true, } @@ -283,10 +281,9 @@ func TestConfigReload(t *testing.T) { MaxPingsOut: 2, WriteDeadline: 10 * time.Second, Cluster: ClusterOpts{ - Name: "abc", - Host: "127.0.0.1", - Port: server.ClusterAddr().Port, - Compression: CompressionOpts{Mode: CompressionS2Auto, RTTThresholds: defaultCompressionS2AutoRTTThresholds}, + Name: "abc", + Host: "127.0.0.1", + Port: server.ClusterAddr().Port, }, NoSigs: true, } @@ -5380,12 +5377,12 @@ func TestConfigReloadRouteCompression(t *testing.T) { %s } ` - conf1 := createConfFile(t, []byte(fmt.Sprintf(tmpl, "A", _EMPTY_, "compression: accept"))) + conf1 := createConfFile(t, []byte(fmt.Sprintf(tmpl, "A", _EMPTY_, _EMPTY_))) s1, o1 := RunServerWithConfig(conf1) defer s1.Shutdown() routes := fmt.Sprintf("routes: [\"nats://127.0.0.1:%d\"]", o1.Cluster.Port) - conf2 := createConfFile(t, []byte(fmt.Sprintf(tmpl, "B", routes, "compression: accept"))) + conf2 := createConfFile(t, []byte(fmt.Sprintf(tmpl, "B", routes, _EMPTY_))) s2, _ := RunServerWithConfig(conf2) defer s2.Shutdown() @@ -5475,25 +5472,23 @@ func TestConfigReloadRouteCompression(t *testing.T) { return nil }) } - // Since both started with "accept", which means that a server can - // accept/switch to compression but not initiate compression, they - // should both be "off" + // Since both started without any compression setting, we default to + // "accept" which means that a server can accept/switch to compression + // but not initiate compression, so they should both be "off" checkCompMode(CompressionOff, CompressionOff, false) - // Now reload s1 with "on" ("auto"), since s2 is *configured* with "accept", - // it won't use "auto" but instead fall back to "fast". S1 should be set - // to "uncompressed" because the default RTT threshold is 10ms and we should - // below that... + // Now relead s1 with "on" (s2_fast), since s2 is *configured* with "accept", + // they should both be CompressionS2Fast, even before we reload s2. reloadUpdateConfig(t, s1, conf1, fmt.Sprintf(tmpl, "A", _EMPTY_, "compression: on")) - checkCompMode(CompressionS2Uncompressed, CompressionS2Fast, true) + checkCompMode(CompressionS2Fast, CompressionS2Fast, true) // Now reload s2 reloadUpdateConfig(t, s2, conf2, fmt.Sprintf(tmpl, "B", routes, "compression: on")) - checkCompMode(CompressionS2Uncompressed, CompressionS2Uncompressed, false) + checkCompMode(CompressionS2Fast, CompressionS2Fast, false) // Move on with "better" reloadUpdateConfig(t, s1, conf1, fmt.Sprintf(tmpl, "A", _EMPTY_, "compression: s2_better")) - // s1 should be at "better", but s2 still at "uncompressed" - checkCompMode(CompressionS2Better, CompressionS2Uncompressed, false) + // s1 should be at "better", but s2 still at "fast" + checkCompMode(CompressionS2Better, CompressionS2Fast, false) // Now reload s2 reloadUpdateConfig(t, s2, conf2, fmt.Sprintf(tmpl, "B", routes, "compression: s2_better")) checkCompMode(CompressionS2Better, CompressionS2Better, false) @@ -5525,16 +5520,16 @@ func TestConfigReloadRouteCompression(t *testing.T) { // S1 should be "best" but S2 should have stayed at "better" checkCompMode(CompressionS2Best, CompressionS2Better, false) - // Change compression setting back to "accept", which in that case we want - // to have a negotiation and use the remote's compression level. So - // connections should be re-created. - reloadUpdateConfig(t, s1, conf1, fmt.Sprintf(tmpl, "A", _EMPTY_, "compression: accept")) + // If we remove the compression setting, it defaults to "accept", which + // in that case we want to have a negotiation and use the remote's compression + // level. So connections should be re-created. + reloadUpdateConfig(t, s1, conf1, fmt.Sprintf(tmpl, "A", _EMPTY_, _EMPTY_)) checkCompMode(CompressionS2Better, CompressionS2Better, true) // To avoid flapping, add a little sleep here to make sure we have things // settled before reloading s2. time.Sleep(100 * time.Millisecond) // And if we do the same with s2, then we will end-up with no compression. - reloadUpdateConfig(t, s2, conf2, fmt.Sprintf(tmpl, "B", routes, "compression: accept")) + reloadUpdateConfig(t, s2, conf2, fmt.Sprintf(tmpl, "B", routes, _EMPTY_)) checkCompMode(CompressionOff, CompressionOff, true) } diff --git a/server/routes_test.go b/server/routes_test.go index 43085041..f3ad4508 100644 --- a/server/routes_test.go +++ b/server/routes_test.go @@ -3157,11 +3157,11 @@ func TestRouteCompressionOptions(t *testing.T) { expected string rtts []time.Duration }{ - {"boolean enabled", "true", nil, CompressionS2Auto, defaultCompressionS2AutoRTTThresholds}, - {"string enabled", "enabled", nil, CompressionS2Auto, defaultCompressionS2AutoRTTThresholds}, - {"string EnaBled", "EnaBled", nil, CompressionS2Auto, defaultCompressionS2AutoRTTThresholds}, - {"string on", "on", nil, CompressionS2Auto, defaultCompressionS2AutoRTTThresholds}, - {"string ON", "ON", nil, CompressionS2Auto, defaultCompressionS2AutoRTTThresholds}, + {"boolean enabled", "true", nil, CompressionS2Fast, nil}, + {"string enabled", "enabled", nil, CompressionS2Fast, nil}, + {"string EnaBled", "EnaBled", nil, CompressionS2Fast, nil}, + {"string on", "on", nil, CompressionS2Fast, nil}, + {"string ON", "ON", nil, CompressionS2Fast, nil}, {"string fast", "fast", nil, CompressionS2Fast, nil}, {"string Fast", "Fast", nil, CompressionS2Fast, nil}, {"string s2_fast", "s2_fast", nil, CompressionS2Fast, nil}, @@ -3238,7 +3238,7 @@ func TestRouteCompressionOptions(t *testing.T) { } }) } - // Test that with no compression specified, we default to "auto" + // Test that with no compression specified, we default to "accept" conf := createConfFile(t, []byte(` port: -1 cluster { @@ -3247,8 +3247,8 @@ func TestRouteCompressionOptions(t *testing.T) { `)) s, o := RunServerWithConfig(conf) defer s.Shutdown() - if o.Cluster.Compression.Mode != CompressionS2Auto { - t.Fatalf("Expected compression value to be %q, got %q", CompressionS2Auto, o.Cluster.Compression.Mode) + if o.Cluster.Compression.Mode != CompressionAccept { + t.Fatalf("Expected compression value to be %q, got %q", CompressionAccept, o.Cluster.Compression.Mode) } for _, test := range []struct { name string @@ -3307,7 +3307,7 @@ func TestRouteCompression(t *testing.T) { cluster { name: "local" port: -1 - compression: s2_fast + compression: true pool_size: %d %s %s @@ -3446,25 +3446,25 @@ func TestRouteCompressionMatrixModes(t *testing.T) { {"accept off", "accept", "off", CompressionOff, CompressionOff}, {"accept accept", "accept", "accept", CompressionOff, CompressionOff}, - {"accept on", "accept", "on", CompressionS2Fast, CompressionS2Uncompressed}, + {"accept on", "accept", "on", CompressionS2Fast, CompressionS2Fast}, {"accept better", "accept", "better", CompressionS2Better, CompressionS2Better}, {"accept best", "accept", "best", CompressionS2Best, CompressionS2Best}, {"on off", "on", "off", CompressionOff, CompressionOff}, - {"on accept", "on", "accept", CompressionS2Uncompressed, CompressionS2Fast}, - {"on on", "on", "on", CompressionS2Uncompressed, CompressionS2Uncompressed}, - {"on better", "on", "better", CompressionS2Uncompressed, CompressionS2Better}, - {"on best", "on", "best", CompressionS2Uncompressed, CompressionS2Best}, + {"on accept", "on", "accept", CompressionS2Fast, CompressionS2Fast}, + {"on on", "on", "on", CompressionS2Fast, CompressionS2Fast}, + {"on better", "on", "better", CompressionS2Fast, CompressionS2Better}, + {"on best", "on", "best", CompressionS2Fast, CompressionS2Best}, {"better off", "better", "off", CompressionOff, CompressionOff}, {"better accept", "better", "accept", CompressionS2Better, CompressionS2Better}, - {"better on", "better", "on", CompressionS2Better, CompressionS2Uncompressed}, + {"better on", "better", "on", CompressionS2Better, CompressionS2Fast}, {"better better", "better", "better", CompressionS2Better, CompressionS2Better}, {"better best", "better", "best", CompressionS2Better, CompressionS2Best}, {"best off", "best", "off", CompressionOff, CompressionOff}, {"best accept", "best", "accept", CompressionS2Best, CompressionS2Best}, - {"best on", "best", "on", CompressionS2Best, CompressionS2Uncompressed}, + {"best on", "best", "on", CompressionS2Best, CompressionS2Fast}, {"best better", "best", "better", CompressionS2Best, CompressionS2Better}, {"best best", "best", "best", CompressionS2Best, CompressionS2Best}, } { diff --git a/server/server.go b/server/server.go index b319852b..7181e058 100644 --- a/server/server.go +++ b/server/server.go @@ -374,7 +374,7 @@ func validateAndNormalizeCompressionOption(c *CompressionOpts) error { c.Mode = CompressionOff case "accept": c.Mode = CompressionAccept - case "on", "enabled", "true", "auto", "s2_auto": + case "auto", "s2_auto": var rtts []time.Duration if len(c.RTTThresholds) == 0 { rtts = defaultCompressionS2AutoRTTThresholds @@ -420,7 +420,7 @@ func validateAndNormalizeCompressionOption(c *CompressionOpts) error { } c.Mode = CompressionS2Auto c.RTTThresholds = rtts - case "fast", "s2_fast": + case "on", "enabled", "true", "fast", "s2_fast": c.Mode = CompressionS2Fast case "better", "s2_better": c.Mode = CompressionS2Better