Skip to content

Improve the code readability #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Totktonada opened this issue Nov 22, 2019 · 0 comments
Open

Improve the code readability #30

Totktonada opened this issue Nov 22, 2019 · 0 comments
Labels
code health Improve code readability, simplify maintenance and so on good first issue Good for newcomers

Comments

@Totktonada
Copy link
Member

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.

@Totktonada Totktonada added the code health Improve code readability, simplify maintenance and so on label Nov 22, 2019
@Totktonada Totktonada added the good first issue Good for newcomers label Aug 18, 2020
@Totktonada Totktonada added 5sp backlog Issues for upcoming releases (to be done soon) prio4 labels Dec 21, 2020
@LeonidVas LeonidVas removed backlog Issues for upcoming releases (to be done soon) prio4 labels Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Improve code readability, simplify maintenance and so on good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants