Skip to content

Commit 930d5eb

Browse files
tfogoskriptble
authored andcommitted
GODRIVER-836 Fix zLib compression
Fixes the CompressBytes method of ZlibCompressor to correctly return a slice of compressed bytes. Also ensuresUncompressBytes will correctly return a slice of uncompressed bytes. Previously both methods would return slices of length 0. New Compressors are initialized for every connection to avoid race conditions with the zLib compressor. Evergreen config is updated to run zlib tests for mongo versions > 3.4. Snappy tests now only run for versions > 3.2. Unit tests are added for compressors. Change-Id: I4524027a1ed85eb698a75e528b56cfe56a4d6167
1 parent c1b9339 commit 930d5eb

File tree

10 files changed

+337
-118
lines changed

10 files changed

+337
-118
lines changed

.evergreen/config.yml

Lines changed: 72 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,8 @@ tasks:
394394
AUTH: "noauth"
395395
SSL: "nossl"
396396

397-
- name: test-standalone-noauth-nossl-compression
398-
tags: ["test", "standalone", "compression"]
397+
- name: test-standalone-noauth-nossl-snappy-compression
398+
tags: ["test", "standalone", "compression", "snappy"]
399399
commands:
400400
- func: bootstrap-mongo-orchestration
401401
vars:
@@ -409,6 +409,21 @@ tasks:
409409
SSL: "nossl"
410410
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
411411

412+
- name: test-standalone-noauth-nossl-zlib-compression
413+
tags: ["test", "standalone", "compression", "zlib"]
414+
commands:
415+
- func: bootstrap-mongo-orchestration
416+
vars:
417+
TOPOLOGY: "server"
418+
AUTH: "noauth"
419+
SSL: "nossl"
420+
- func: run-tests
421+
vars:
422+
TOPOLOGY: "server"
423+
AUTH: "noauth"
424+
SSL: "nossl"
425+
MONGO_GO_DRIVER_COMPRESSOR: "zlib"
426+
412427
- name: test-standalone-auth-ssl
413428
tags: ["test", "standalone", "authssl"]
414429
commands:
@@ -423,8 +438,8 @@ tasks:
423438
AUTH: "auth"
424439
SSL: "ssl"
425440

426-
- name: test-standalone-auth-ssl-compression
427-
tags: ["test", "standalone", "authssl", "compression"]
441+
- name: test-standalone-auth-ssl-snappy-compression
442+
tags: ["test", "standalone", "authssl", "compression", "snappy"]
428443
commands:
429444
- func: bootstrap-mongo-orchestration
430445
vars:
@@ -438,22 +453,23 @@ tasks:
438453
SSL: "ssl"
439454
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
440455

441-
- name: test-replicaset-noauth-nossl
442-
tags: ["test", "replicaset"]
456+
- name: test-standalone-auth-ssl-zlib-compression
457+
tags: ["test", "standalone", "authssl", "compression", "zlib"]
443458
commands:
444459
- func: bootstrap-mongo-orchestration
445460
vars:
446-
TOPOLOGY: "replica_set"
447-
AUTH: "noauth"
448-
SSL: "nossl"
461+
TOPOLOGY: "server"
462+
AUTH: "auth"
463+
SSL: "ssl"
449464
- func: run-tests
450465
vars:
451-
TOPOLOGY: "replica_set"
452-
AUTH: "noauth"
453-
SSL: "nossl"
466+
TOPOLOGY: "server"
467+
AUTH: "auth"
468+
SSL: "ssl"
469+
MONGO_GO_DRIVER_COMPRESSOR: "zlib"
454470

455-
- name: test-replicaset-noauth-nossl-compression
456-
tags: ["test", "replicaset", "compression"]
471+
- name: test-replicaset-noauth-nossl
472+
tags: ["test", "replicaset"]
457473
commands:
458474
- func: bootstrap-mongo-orchestration
459475
vars:
@@ -465,7 +481,6 @@ tasks:
465481
TOPOLOGY: "replica_set"
466482
AUTH: "noauth"
467483
SSL: "nossl"
468-
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
469484

470485
- name: test-replicaset-auth-ssl
471486
tags: ["test", "replicaset", "authssl"]
@@ -481,23 +496,22 @@ tasks:
481496
AUTH: "auth"
482497
SSL: "ssl"
483498

484-
- name: test-replicaset-auth-ssl-compression
485-
tags: ["test", "replicaset", "authssl", "compression"]
499+
- name: test-sharded-noauth-nossl
500+
tags: ["test", "sharded"]
486501
commands:
487502
- func: bootstrap-mongo-orchestration
488503
vars:
489-
TOPOLOGY: "replica_set"
490-
AUTH: "auth"
491-
SSL: "ssl"
504+
TOPOLOGY: "sharded_cluster"
505+
AUTH: "noauth"
506+
SSL: "nossl"
492507
- func: run-tests
493508
vars:
494-
TOPOLOGY: "replica_set"
495-
AUTH: "auth"
496-
SSL: "ssl"
497-
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
509+
TOPOLOGY: "sharded_cluster"
510+
AUTH: "noauth"
511+
SSL: "nossl"
498512

499-
- name: test-sharded-noauth-nossl
500-
tags: ["test", "sharded"]
513+
- name: test-sharded-noauth-nossl-snappy-compression
514+
tags: ["test", "sharded", "compression", "snappy"]
501515
commands:
502516
- func: bootstrap-mongo-orchestration
503517
vars:
@@ -509,9 +523,10 @@ tasks:
509523
TOPOLOGY: "sharded_cluster"
510524
AUTH: "noauth"
511525
SSL: "nossl"
526+
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
512527

513-
- name: test-sharded-noauth-nossl-compression
514-
tags: ["test", "sharded", "compression"]
528+
- name: test-sharded-noauth-nossl-zlib-compression
529+
tags: ["test", "sharded", "compression", "zlib"]
515530
commands:
516531
- func: bootstrap-mongo-orchestration
517532
vars:
@@ -523,7 +538,7 @@ tasks:
523538
TOPOLOGY: "sharded_cluster"
524539
AUTH: "noauth"
525540
SSL: "nossl"
526-
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
541+
MONGO_GO_DRIVER_COMPRESSOR: "zlib"
527542

528543
- name: test-sharded-auth-ssl
529544
tags: ["test", "sharded", "authssl"]
@@ -539,8 +554,8 @@ tasks:
539554
AUTH: "auth"
540555
SSL: "ssl"
541556

542-
- name: test-sharded-auth-ssl-compression
543-
tags: ["test", "sharded", "authssl", "compression"]
557+
- name: test-sharded-auth-ssl-snappy-compression
558+
tags: ["test", "sharded", "authssl", "compression", "snappy"]
544559
commands:
545560
- func: bootstrap-mongo-orchestration
546561
vars:
@@ -554,27 +569,32 @@ tasks:
554569
SSL: "ssl"
555570
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
556571

557-
- name: test-enterprise-auth-plain
558-
tags: ["test", "enterprise-auth"]
572+
- name: test-sharded-auth-ssl-zlib-compression
573+
tags: ["test", "sharded", "authssl", "compression", "zlib"]
559574
commands:
560-
- func: run-enterprise-auth-tests
575+
- func: bootstrap-mongo-orchestration
561576
vars:
562-
MONGODB_URI: "${plain_auth_mongodb_uri}"
577+
TOPOLOGY: "sharded_cluster"
578+
AUTH: "auth"
579+
SSL: "ssl"
580+
- func: run-tests
581+
vars:
582+
TOPOLOGY: "sharded_cluster"
583+
AUTH: "auth"
584+
SSL: "ssl"
585+
MONGO_GO_DRIVER_COMPRESSOR: "zlib"
563586

564-
- name: test-enterprise-auth-plain-compression
587+
- name: test-enterprise-auth-plain
565588
tags: ["test", "enterprise-auth"]
566589
commands:
567590
- func: run-enterprise-auth-tests
568591
vars:
569592
MONGODB_URI: "${plain_auth_mongodb_uri}"
570-
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
571593

572594
- name: test-enterprise-auth-gssapi
573595
tags: ["test", "enterprise-auth"]
574596
commands:
575597
- func: run-enterprise-gssapi-auth-tests
576-
vars:
577-
MONGO_GO_DRIVER_COMPRESSOR: "snappy"
578598

579599
- name: test-enterprise-auth-gssapi-service-host
580600
tags: ["test", "enterprise-auth"]
@@ -730,7 +750,19 @@ buildvariants:
730750
- name: ".test !.authssl !.enterprise-auth !.compression"
731751

732752
- matrix_name: "tests-nonlegacy-servers"
733-
matrix_spec: { version: ["3.2", "3.4", "3.6", "4.0", "latest"], os-ssl-32: "*" }
753+
matrix_spec: { version: "3.2", os-ssl-32: "*" }
754+
display_name: "${version} ${os-ssl-32}"
755+
tasks:
756+
- name: ".test !.enterprise-auth !.compression"
757+
758+
- matrix_name: "tests-nonlegacy-servers-with-snappy-support"
759+
matrix_spec: { version: "3.4", os-ssl-32: "*" }
760+
display_name: "${version} ${os-ssl-32}"
761+
tasks:
762+
- name: ".test !.enterprise-auth !.zlib"
763+
764+
- matrix_name: "tests-nonlegacy-servers-with-zlib-support"
765+
matrix_spec: { version: ["3.6", "4.0", "latest"], os-ssl-32: "*" }
734766
display_name: "${version} ${os-ssl-32}"
735767
tasks:
736768
- name: ".test !.enterprise-auth"

mongo/client.go

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,9 @@ import (
2424
"go.mongodb.org/mongo-driver/x/mongo/driver/topology"
2525
"go.mongodb.org/mongo-driver/x/mongo/driver/uuid"
2626
"go.mongodb.org/mongo-driver/x/network/command"
27-
"go.mongodb.org/mongo-driver/x/network/compressor"
2827
"go.mongodb.org/mongo-driver/x/network/connection"
2928
"go.mongodb.org/mongo-driver/x/network/connstring"
3029
"go.mongodb.org/mongo-driver/x/network/description"
31-
"go.mongodb.org/mongo-driver/x/network/wiremessage"
3230
)
3331

3432
const defaultLocalThreshold = 15 * time.Millisecond
@@ -209,40 +207,30 @@ func (c *Client) configure(opts *options.ClientOptions) error {
209207
appName = *opts.AppName
210208
}
211209
// Compressors & ZlibLevel
212-
var compressors []string
210+
var comps []string
213211
if len(opts.Compressors) > 0 {
214-
compressors = opts.Compressors
215-
comps := make([]compressor.Compressor, 0, len(compressors))
216-
217-
for _, c := range compressors {
218-
switch c {
219-
case "snappy":
220-
comps = append(comps, compressor.CreateSnappy())
221-
case "zlib":
222-
level := wiremessage.DefaultZlibLevel
223-
if opts.ZlibLevel != nil {
224-
level = *opts.ZlibLevel
225-
}
226-
zlibComp, err := compressor.CreateZlib(level)
227-
if err != nil {
228-
return err
229-
}
230-
comps = append(comps, zlibComp)
231-
}
232-
}
212+
comps = opts.Compressors
233213

234214
connOpts = append(connOpts, connection.WithCompressors(
235-
func(compressors []compressor.Compressor) []compressor.Compressor {
215+
func(compressors []string) []string {
236216
return append(compressors, comps...)
237217
},
238218
))
239219

220+
for _, comp := range comps {
221+
if comp == "zlib" {
222+
connOpts = append(connOpts, connection.WithZlibLevel(func(level *int) *int {
223+
return opts.ZlibLevel
224+
}))
225+
}
226+
}
227+
240228
serverOpts = append(serverOpts, topology.WithCompressionOptions(
241-
func(opts ...string) []string { return append(opts, compressors...) },
229+
func(opts ...string) []string { return append(opts, comps...) },
242230
))
243231
}
244232
// Handshaker
245-
var handshaker connection.Handshaker = &command.Handshake{Client: command.ClientDoc(appName), Compressors: compressors}
233+
var handshaker connection.Handshaker = &command.Handshake{Client: command.ClientDoc(appName), Compressors: comps}
246234
// Auth & Database & Password & Username
247235
if opts.Auth != nil {
248236
cred := &auth.Cred{
@@ -271,7 +259,7 @@ func (c *Client) configure(opts *options.ClientOptions) error {
271259
handshakeOpts := &auth.HandshakeOptions{
272260
AppName: appName,
273261
Authenticator: authenticator,
274-
Compressors: compressors,
262+
Compressors: comps,
275263
}
276264
if mechanism == "" {
277265
// Required for SASL mechanism negotiation during handshake

x/mongo/driver/topology/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ var ErrServerClosed = errors.New("server is closed")
3939
var ErrServerConnected = errors.New("server is connected")
4040

4141
// SelectedServer represents a specific server that was selected during server selection.
42-
// It contains the kind of the typology it was selected from.
42+
// It contains the kind of the topology it was selected from.
4343
type SelectedServer struct {
4444
*Server
4545

x/mongo/driver/topology/topology_options.go

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"go.mongodb.org/mongo-driver/x/mongo/driver/auth"
1515
"go.mongodb.org/mongo-driver/x/network/command"
16-
"go.mongodb.org/mongo-driver/x/network/compressor"
1716
"go.mongodb.org/mongo-driver/x/network/connection"
1817
"go.mongodb.org/mongo-driver/x/network/connstring"
1918
)
@@ -191,26 +190,18 @@ func WithConnString(fn func(connstring.ConnString) connstring.ConnString) Option
191190
}
192191

193192
if len(cs.Compressors) > 0 {
194-
comp := make([]compressor.Compressor, 0, len(cs.Compressors))
195-
196-
for _, c := range cs.Compressors {
197-
switch c {
198-
case "snappy":
199-
comp = append(comp, compressor.CreateSnappy())
200-
case "zlib":
201-
zlibComp, err := compressor.CreateZlib(cs.ZlibLevel)
202-
if err != nil {
203-
return err
204-
}
193+
connOpts = append(connOpts, connection.WithCompressors(func(compressors []string) []string {
194+
return append(compressors, cs.Compressors...)
195+
}))
205196

206-
comp = append(comp, zlibComp)
197+
for _, comp := range cs.Compressors {
198+
if comp == "zlib" {
199+
connOpts = append(connOpts, connection.WithZlibLevel(func(level *int) *int {
200+
return &cs.ZlibLevel
201+
}))
207202
}
208203
}
209204

210-
connOpts = append(connOpts, connection.WithCompressors(func(compressors []compressor.Compressor) []compressor.Compressor {
211-
return append(compressors, comp...)
212-
}))
213-
214205
c.serverOpts = append(c.serverOpts, WithCompressionOptions(func(opts ...string) []string {
215206
return append(opts, cs.Compressors...)
216207
}))

0 commit comments

Comments
 (0)