Skip to content

Improve the code readability #30

Open
@Totktonada

Description

@Totktonada

In brief: there are a lot of details that neither documented, nor obvious from the code. Any change is painful so. We should provide those details as comments in the code and make appropriate renaming.

I'll share my observations below and hopefully will come back later to actually make proper changes.


<connection object>.queue is not queue, it is a fiber channel with one item at max. It is used for synchronization of using a connection betweeb fiber: :get() is acquiring a lock, :put() is releasing it. Aside of that, a value in this channel is an indicator whether a connection is healthy: :put(true) is used to mark it as healthy and :put(false) to mark it as broken.

A connection is marked as broken when any request fails due to any reason (except when :ping() fails, but it looks more as a mistake).

<connection object>.usable is another marker. When a connection is closed or is put back to a pool it marked as unusable. It is not the same that broken connection.

<pool object>.queue is also not a queue, it is also a fiber channel, which works as fixed-size pool of prespawned raw connection objects (see below about their naming), but also can contain nil markers. This channel contains <pool object>.size elements at max: when the channel is full we can acquire this amount of connections, when it is empty <pool object>:get() will wait until someone will do <pool object>:put(). When a connection is put back to a pool we'll add its raw connection to the pool if it is not broken and is not unusable. Otherwise we'll add nil marker to the pool's and discard the raw connection. We handle this marker at <pool object>:get(): the new raw connection will be created at this moment.

<pool object>.usable is just whether the pool is closed.

About raw connections mentioned above. A connection contains conn field that holds underlying connection object from driver.c. Local variables that hold this field (acquired from driver.connect(), <connection object>.conn or via <pool object>.queue:get()) has no strict naming: they are conn, mysqlconn or mysql_conn. Let's use one name for one term. We should name the field and all local variables in the same way.

The connector handles the case when a connection was not put back to a pool and a user lost it at all: say a fiber, which did acquire the connection was cancelled. In this case the raw connection will be properly closed and nil marker will be added to the pool: a kind of implicit <pool object>:put() if a connection was lost.

Metadata

Metadata

Assignees

No one assigned

    Labels

    code healthImprove code readability, simplify maintenance and so ongood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions