search

DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.83k stars 1.08k forks source link

Fixed goroutine leaks and deadlocks in Worker and Resp Integrations Tests #1298

Closed psrvere closed 1 day ago

psrvere commented 2 days ago

This PR fixes a bunch of issues.

First, it fixes goroutine leaks in worker by adding a ctx ub child goroutine to exit if parent is exits. See #1297 for more details.

Second, there were additional goroutine leaks because of initHealthCheck goroutine (watch_connection.go) in SDK. This was because WatchConn was not closed in any of the tests, only Clients were closed. WatchConn.Close() sends closing signal to this goroutine. This is fixed by making helper functions ClosePublisherSubscribers and ClosePublisherSubscribersSDK which close it properly and make is easier for other developers to not worry about it going forward.

Third, none of the tests unsubscribe as part of cleanup. Hence, watch manager keeps sending updates to buffered channel AdhocChannel which has a configured capacity of 20. At some point, it is filled and new watch tests keep on waiting on this channel. This is fixed for GET.WATCH and GET.UNWATCH tests by adding helper functions unsubscribeFromUpdates and unsubscribeFromUpdatesSDK which sets the right cleanup convention for other devs.

This is not fixed for PFCOUNT.WATCH and ZRANGE.WATCH tests as their unwatch functionality is still not implemented. TODO has been added in the respective places.

This PR also makes a few minor changes like acquiring connection in TestMain just before firing ABORT command, migrating testing library to testify/assert, etc.

Fixes #1297

psrvere commented 2 days ago

@JyotinderSingh - Please review.

psrvere commented 2 days ago

@soumya-codes @JyotinderSingh - I have addressed all comments. Please check.