Connect your Go app to a group of database Nodes without the need of a SQL aware proxy! Multidb provides a SQL Database multiplexer for parallel queries using Go routines, boosting performance and availabilty.
In #11, the connErr field was removed. Also, re-connection logic is bound to be removed (#14). By implementing the possibility of adding an error callback function during Node creation, package consumers would be able to monitor and log Node specific errors. As those errors might never get reported during MultiQuery calls, if at least one query on a Node succeeds.
At the moment, if a context is canceled by the caller, we are counting it as a connection error for the node. After enough of such cancels, the node is disregarded.
Package context defines the Canceledvariable, against which we can test.
// ErrNoNodes is returned when there are no connected nodes available for the requested operation
ErrNoNodes="No available nodes"
// ErrNoMaster is returned when no master is available
ErrNoMaster="No available master, cause: %w"
// ErrSuccesReq is returned when higher than 1.0
ErrSuccesReq="SuccesReq > 1"
)
The more idiomatic way is to declare global variables, using errors.New(). At the moment the errors are created inside the function scope. This means equality can only be tested based on strings. Instead, this package should offer errors which are easily investigated using errors.Is().
Reconnection is handled by the SQL driver. Also, if an error is returned on first Connect, it mostly mean an error was made in de dsn, not the actual connection. (SQL library only connects when a Connection needs to be Acquired the first time).
Since Go 1.13, errors can be wrapped. Helper methods are provided to "Unwrap" and test for the originating error. I would like to refactor this package to use the Is method from the errors package, in order to provide more robust testing at:
Previously, connections were fully managed by MultiDB. This way it also had full controll over the status of the Master node. However, we are dropping connection management in favor of the standard sql library own (re) connection logic. We have dropped Node specific error statistics in favor of better performance.
This means that, in case the Master starts failing (and another Node has been promoted) this package won't be aware of this. Simply put, if the caller obtains a Master node through MultiDB.Master() or MultiDB.MasterTX(), it should decide itself if a returned error is serious enough to find a new Master.
I would like to add a utility function that can be called in case of errors, that would help with this decision making and optionally even triggering SelectMaster() if needed.
After we drop re-connection logic (#14), it might be feasible to greatly simplify or remove the drivers.Configurator interface. Perhaps make it possible to set DB objects directly. This way we can support all driver implementations without (too much) hassle.
Overwriting the DB potentially leaves it dangling with open connections in its pool and it will not get garbage collected. In the long run, this can lead to memory leaks, if the caller does not keep track of re-configured databases and proper closing of old entities.
Opening and Closing of DB is not in the scope of this packages. However, we can aide the process by returning the discarded DB from mdb.Add(), so that downstream can choose to close it.
// Errors produced by calling DB directly are not monitored by this package.
DB*sql.DB
connErrerror
reconnectWait time.Duration
reconnectingbool
mtx sync.RWMutex
}
Currently the last encountered error on a DB Node is stored in the connErr field. The last occurred error can be received by the ConnErr function. Recently, Go vet started to complain about reflect.DeepEqual on the error in unit test.
Practically speaking:
Setting and getting of the error added complexity to the code and it would be cleaner without
If a query cannot complete due to multiple or similar errors, an error is properly returned
I don't see a practical point where package consumers will investigate individual Nodes after queries. This would only add in code complexity on the caller's side
Queries on a Node are executed concurrently. A mutex is preventing race conditions on the connErr, however it does not guarantee which error is returned from the ConnErr method. A caller might be expecting the error status of the latest called Query, but it may was well return the latest terminated query. There is no way query calls are matched against the connErr state. Meaning this can lead to misunderstanding of the information and bugs.
It would be a "cleaner" impression of the project if we could move this in a sub-module for testing only. Then, we let CI bump the sub-module to the latest commit hash on every push to verify compliance.