watermill
watermill copied to clipboard
[watermill-sql] Backoff Manager does not handle concurrent update for PostgreSQL
In the watermill-sql backoff manager there is code to detect deadlock
errors (https://github.com/ThreeDotsLabs/watermill-sql/blob/master/pkg/sql/backoff_manager.go#L37). This code does not handle concurrent update
errors for PostgreSQL.
I get the following error:
[watermill] 2021/03/03 15:17:09.093600 backoff_manager.go:46: level=ERROR msg="Error querying for message" consumer_group=consumer err="could not unmarshal message from query: could not scan message row: pq: could not serialize access due to concurrent update" message_uuid= subscriber_id=01EZW75WX7VXCFNZ96TYV6WW6K topic=events wait_time=1s
The problem can be solved by extending the condition as follows (https://github.com/ThreeDotsLabs/watermill-sql/blob/master/pkg/sql/backoff_manager.go#L37):
if strings.Contains(strings.ToLower(err.Error()), "deadlock") || strings.Contains(strings.ToLower(err.Error()), "concurrent update") {
The question is, what is the best approach to address this issue. I assume, the error messages, that are considered a deadlock are highly database (or even version) dependent. Therefore I propose to delegate this filter logic (implementation of BackoffManager
interface) to the database specific implementation. I am happy to provide a PR which provides separate BackoffManager implementations for Mysql (the current default implementation) and PostgreSQL.
An other approach would be to accept the strings, that are matched against the errors, as argument for NewDefaultBackoffManager
.
Hey @breml! I think both approaches sound good / better than the current solution. I'm happy to review a PR.
Hello, had this issue too. I would like to see this merged.
Oh, it appears in the v1.3.4, awesome.
@breml Thank you!