Refactor NewAerospikeBackend function #164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexBVolcy
reviewed
Mar 11, 2024
There was a problem hiding this comment.
I'm getting this error when I run package tests through VS code, which I utilize to check test coverage:
time="2024-03-11T17:23:43-04:00" level=info msg="config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts"
time="2024-03-11T17:23:43-04:00" level=info msg="config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts"
time="2024-03-11T17:23:43-04:00" level=info msg="config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts"
time="2024-03-11T17:23:43-04:00" level=info msg="config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts"
time="2024-03-11T17:24:13-04:00" level=fatal msg="Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: <nil>: command execution timed out on client: See `Policy.Timeout`"
time="2024-03-11T17:24:13-04:00" level=info msg="config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts"
coverage: 8.4% of statements
panic: test timed out after 30s
running tests:
TestNewAerospikeBackend (30s)
goroutine 25 [running]:
testing.(*M).startAlarm.func1()
/usr/local/go/src/testing/testing.go:2259 +0x320
created by time.goFunc
/usr/local/go/src/time/sleep.go:176 +0x38
goroutine 1 [chan receive]:
testing.(*T).Run(0x14000159d40, {0x1031b28ca?, 0x24a658620ab25?}, 0x103411370)
/usr/local/go/src/testing/testing.go:1649 +0x350
testing.runTests.func1(0x14000159d40?)
/usr/local/go/src/testing/testing.go:2054 +0x48
testing.tRunner(0x14000159d40, 0x140001bbc28)
/usr/local/go/src/testing/testing.go:1595 +0xe8
testing.runTests(0x140001d3860?, {0x10386c940, 0x13, 0x13}, {0x40?, 0x1033b2180?, 0x103874fa0?})
/usr/local/go/src/testing/testing.go:2052 +0x3b4
testing.(*M).Run(0x140001d3860)
/usr/local/go/src/testing/testing.go:1925 +0x538
main.main()
_testmain.go:117 +0x1c8
goroutine 23 [select]:
github.com/aerospike/aerospike-client-go/v6.(*Cluster).waitTillStabilized(0x140000da000)
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:515 +0xf8
github.com/aerospike/aerospike-client-go/v6.NewCluster(0x1400014faf8, {0x140000b6010, 0x1, 0x1})
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:144 +0x55c
github.com/aerospike/aerospike-client-go/v6.NewClientWithPolicyAndHost(0x0?, {0x140000b6010, 0x1, 0x1})
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/client.go:88 +0xac
github.com/prebid/prebid-cache/backends.NewAerospikeBackend({0x0, {0x1031a869e, 0x7}, {0x0, 0x0, 0x0}, 0x22b8, {0x0, 0x0}, {0x0, ...}, ...}, ...)
/Users/alexvolcy/Desktop/prebid-cache/backends/aerospike.go:60 +0x1c0
github.com/prebid/prebid-cache/backends.TestNewAerospikeBackend.func3()
/Users/alexvolcy/Desktop/prebid-cache/backends/aerospike_test.go:253 +0x4c
github.com/stretchr/testify/assert.didPanic(0x103df1b98?)
/Users/alexvolcy/Desktop/pkg/mod/github.com/stretchr/testify@v1.7.1/assert/assertions.go:1018 +0x7c
github.com/stretchr/testify/assert.Panics({0x103415be8, 0x140001ef860}, 0x140000ba0b0, {0x1400014fde0, 0x1, 0x1})
/Users/alexvolcy/Desktop/pkg/mod/github.com/stretchr/testify@v1.7.1/assert/assertions.go:1032 +0x64
github.com/prebid/prebid-cache/backends.TestNewAerospikeBackend(0x140001c60a8?)
/Users/alexvolcy/Desktop/prebid-cache/backends/aerospike_test.go:253 +0x274
testing.tRunner(0x140001ef860, 0x103411370)
/usr/local/go/src/testing/testing.go:1595 +0xe8
created by testing.(*T).Run in goroutine 1
/usr/local/go/src/testing/testing.go:1648 +0x33c
goroutine 24 [select]:
github.com/aerospike/aerospike-client-go/v6.(*Cluster).seedNodes(0x14000149440)
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:607 +0x278
github.com/aerospike/aerospike-client-go/v6.(*Cluster).tend(0x14000149440)
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:249 +0x88
github.com/aerospike/aerospike-client-go/v6.(*Cluster).waitTillStabilized.func1()
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:492 +0x4c
created by github.com/aerospike/aerospike-client-go/v6.(*Cluster).waitTillStabilized in goroutine 23
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:489 +0xac
goroutine 34 [select]:
github.com/aerospike/aerospike-client-go/v6.(*Cluster).seedNodes(0x140000da000)
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:607 +0x278
github.com/aerospike/aerospike-client-go/v6.(*Cluster).tend(0x140000da000)
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:249 +0x88
github.com/aerospike/aerospike-client-go/v6.(*Cluster).waitTillStabilized.func1()
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:492 +0x4c
created by github.com/aerospike/aerospike-client-go/v6.(*Cluster).waitTillStabilized in goroutine 23
/Users/alexvolcy/Desktop/pkg/mod/github.com/aerospike/aerospike-client-go/v6@v6.7.0/cluster.go:489 +0xac
FAIL github.com/prebid/prebid-cache/backends 30.215s
FAIL
I think the error is referring to an inability to connect in Aerospike server maybe because of the time limit. Can this be updated?
|
|
||
| // Aerospike's default connection queue size per node is 256. | ||
| // If cfg.ConnQueueSize is greater than zero, it will override the default. | ||
| if cfg.ConnQueueSize > 0 { |
There was a problem hiding this comment.
I don't think this comment is necessary, the well-written code explains it I think.
There was a problem hiding this comment.
Agree. Probably a leftover. I sometimes structure logic into comments before writing the actual code. Removed.
backends/aerospike.go
Outdated
| clientPolicy.Password = cfg.Password | ||
|
|
||
| // Aerospike's connection idle deadline default is 55 seconds. If greater than zero, this | ||
| // value will override |
There was a problem hiding this comment.
I think you can remove the if greater than zero, this value will override from this comment
AlexBVolcy
previously approved these changes
Mar 14, 2024
VeronikaSolovei9
previously approved these changes
Mar 18, 2024
guscarreon
dismissed stale reviews from VeronikaSolovei9 and AlexBVolcy
via
accac66
AlexBVolcy
approved these changes
Mar 21, 2024
VeronikaSolovei9
approved these changes
Mar 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #162 documents a
NewAerospikeBackendunit test that fails. When looking into the issue documented in issue #162 the need to refactorNewAerospikeBackend, for more accurate testing and better readability, was evident. This pull request makes such modifications.