142 Stop Heartbeat StopAllConsuming()

[wellle]

Once all consumers of the connection have been stopped we also stop the heartbeat goroutine to avoid a goroutine leak.

Close #142. Also related to #149.

[github-actions]

Lines Of Code

Language	Files	Lines	Code	Comments	Blanks	Complexity	Bytes
Go	29	3072 (+22)	2241 (+7)	303 (+10)	528 (+5)	401	82.9K (+1.3K)
Go (test)	8	2477 (+32)	1978 (+17)	88 (+6)	411 (+9)	160 (+1)	73K (+1K)
Markdown	1	657 (+3)	487 (+2)	0	170 (+1)	0	24.8K (+163B)

[github-actions]

Go API Changes

# github.com/adjust/rmq/v5
## compatible changes
(*TestQueue).Remove: added
(*TestQueue).RemoveBytes: added
Queue.Remove: added
Queue.RemoveBytes: added

# summary
Inferred base version: v5.2.0
Suggested version: v5.3.0

[github-actions]

Unit Test Coverage

total: (statements) 71.1%
changed lines: (statements) 80.0%, coverage is less than 90.0%, consider testing the changes more thoroughly

Coverage of changed lines

File	Function	Coverage
Total		80.0%
connection.go		80.0%
connection.go:102	openConnection	100.0%
connection.go:222	StopAllConsuming	100.0%
connection.go:342	stopHeartbeat	100.0%
connection.go:137	heartbeat	20.0%

Coverage diff with base branch

File	Function	Base Coverage	Current Coverage
Total		70.7%	71.1% (+0.4%)
github.com/adjust/rmq/v5/connection.go	heartbeat	41.2%	38.9% (-2.3%)
github.com/adjust/rmq/v5/queue.go	batchTimeout	84.6%	92.3% (+7.7%)
github.com/adjust/rmq/v5/test_batch_consumer.go	Consume	85.7%	100.0% (+14.3%)

[github-actions]

Benchmark Result

Benchmark diff with base branch

name     old time/op    new time/op    delta
Queue-4     185µs ± 1%     184µs ± 2%   ~     (p=0.093 n=6+6)

name     old alloc/op   new alloc/op   delta
Queue-4    35.6kB ± 3%    35.3kB ± 6%   ~     (p=0.589 n=6+6)

name     old allocs/op  new allocs/op  delta
Queue-4      94.0 ± 0%      94.2 ± 1%   ~     (p=0.667 n=4+6)

Benchmark result

name     time/op
Queue-4   184µs ± 2%

name     alloc/op
Queue-4  35.3kB ± 6%

name     allocs/op
Queue-4    94.2 ± 1%

[ahmedaabouzied]

I consistently now run into these test failures on master and on this branch too.

❯ git branch --show-current                                                                                                                                                                                     7s Py base
142-stop-heartbeat
❯ go test ./...                                                                                                                                                                                                    
?       github.com/adjust/rmq/v5/example/batch_consumer [no test files]
?       github.com/adjust/rmq/v5/example/cleaner        [no test files]
?       github.com/adjust/rmq/v5/example/consumer       [no test files]
?       github.com/adjust/rmq/v5/example/handler        [no test files]
?       github.com/adjust/rmq/v5/example/producer       [no test files]
?       github.com/adjust/rmq/v5/example/purger [no test files]
?       github.com/adjust/rmq/v5/example/returner       [no test files]
--- FAIL: TestClusterConnections (0.01s)
    queue_cluster_test.go:38:
                Error Trace:    queue_cluster_test.go:38
                Error:          "[clean-lGYWhZ]" should have 2 item(s), but has 1
                Test:           TestClusterConnections
    queue_cluster_test.go:46:
                Error Trace:    queue_cluster_test.go:46
                Error:          "[clean-lGYWhZ conn2-pJbeaH]" should have 3 item(s), but has 2
                Test:           TestClusterConnections
    queue_cluster_test.go:56:
                Error Trace:    queue_cluster_test.go:56
                Error:          "[clean-lGYWhZ conn2-pJbeaH]" should have 3 item(s), but has 2
                Test:           TestClusterConnections
    queue_cluster_test.go:63:
                Error Trace:    queue_cluster_test.go:63
                Error:          "[clean-lGYWhZ conn2-pJbeaH]" should have 3 item(s), but has 2
                Test:           TestClusterConnections
FAIL
FAIL    github.com/adjust/rmq/v5        1.132s
FAIL

But I think they're not related.

[ahmedaabouzied]

LGTM. The test failures are not related as far as I can see and they also happen on master. Maybe we can fix them separately in a different PR.

[wellle]

Thank you! I have the same test failures when I run these tests locally, it also happens on master. I believe these are Redis Cluster specific tests which only pass if you're using a local Redis Cluster setup.