RejectedSoftware Forums

On 6/21/17 5:43 AM, Paolo Invernizzi wrote:

Vibe.d documentation about lockConnection states:

Note that, after retrieving a connection with lockConnection, the caller has to make sure that the connection is actually physically open and to reopen it if necessary. The ConnectionPool class has no knowledge of the internals of the connection objects.

Let's assume the pool is returning a TCP connection:

• there's no method for 'reopening' it, so it's not possible.
• there's no method to remove that 'closed' connection from the pool.

So, what I've to do with that closed pooled connection? What's the correct way to use a Connection Pool in that case?

Thanks

I don't think you would use a pool for such a setup. My use case is I
don't want to continuously negotiate an open connection with a MySQL
server, so I leave the connections open and just limit them to a certain
number.

A possibility is that the connection has been closed by the other side,
and that was what caused the fiber using it to release it back to the
pool. In that case, you need to open it again.

If your connection type is a "use once" type, you wouldn't use a pool
for it. Of course, the limitation is based on your type -- surely it has
a socket handle internally that can be rebound to a new socket (so a
reopen is technically possible).

-Steve

On 6/26/17 8:58 AM, Paolo Invernizzi wrote:

On Thu, 22 Jun 2017 20:25:40 -0400, Steven Schveighoffer wrote:

I don't think you would use a pool for such a setup. My use case is I
don't want to continuously negotiate an open connection with a MySQL
server, so I leave the connections open and just limit them to a certain
number.

A possibility is that the connection has been closed by the other side,
and that was what caused the fiber using it to release it back to the
pool. In that case, you need to open it again.

If your connection type is a "use once" type, you wouldn't use a pool
for it. Of course, the limitation is based on your type -- surely it has
a socket handle internally that can be rebound to a new socket (so a
reopen is technically possible).

The point is exactly this, if the pool returns a vibe.d TCPConnection, and the server just close it, there's no way to:

• re-open it, there's not method for that in TCPConnection
• inform the pool to just discard it from the pool, as it's closed, and not re-openable

Basically, if some of the pooled connection is closed, the pool suddenly become useless... or at least, as you pointed out, it's not usable with the TCPConnection.
In my opinion, at least a method to tell the pool to discard a pooled connection is needed, what do you think?

It's not a terrible idea, but you could also just wrap the connection in
something that can recreate the TCPConnection, and then pool on that.

I can see it being easier to deal with instructing the pool to discard
or reuse an instance than this.

-Steve

On Mon, 26 Jun 2017 11:24:04 -0400, Steven Schveighoffer wrote:

On 6/26/17 8:58 AM, Paolo Invernizzi wrote:

On Thu, 22 Jun 2017 20:25:40 -0400, Steven Schveighoffer wrote:

I don't think you would use a pool for such a setup. My use case is I
don't want to continuously negotiate an open connection with a MySQL
server, so I leave the connections open and just limit them to a certain
number.

A possibility is that the connection has been closed by the other side,
and that was what caused the fiber using it to release it back to the
pool. In that case, you need to open it again.

If your connection type is a "use once" type, you wouldn't use a pool
for it. Of course, the limitation is based on your type -- surely it has
a socket handle internally that can be rebound to a new socket (so a
reopen is technically possible).

The point is exactly this, if the pool returns a vibe.d TCPConnection, and the server just close it, there's no way to:

• re-open it, there's not method for that in TCPConnection
• inform the pool to just discard it from the pool, as it's closed, and not re-openable

Basically, if some of the pooled connection is closed, the pool suddenly become useless... or at least, as you pointed out, it's not usable with the TCPConnection.
In my opinion, at least a method to tell the pool to discard a pooled connection is needed, what do you think?

It's not a terrible idea, but you could also just wrap the connection in
something that can recreate the TCPConnection, and then pool on that.

I can see it being easier to deal with instructing the pool to discard
or reuse an instance than this.

-Steve

Thanks Steven, that's a good idea.

Am 27.06.2017 um 18:47 schrieb Steven Schveighoffer:

On 6/26/17 2:40 PM, Sönke Ludwig wrote:

In general, nothing speaks against adding a way to discard
connections. It's mostly just that all previous use cases involved
high level connections that had to carry a connection reestablishment
logic anyway, and removing an arbitrary connection is not efficient
for many concurrent connections, because right now they are kept in an
array.

Well, you return them by value anyway. So swapping the removed
connection with the last element should work and be an O(1) operation.

You'll still have to locate the element in O(n).

(Having said that, I've just looked at the code and lockConnection
currently iterates over the array to find a free connection, so the
efficiency argument is kind of moot, and this needs to be improved
anyway.)

You could use a free list if you allocate a pointer along with each
array element.

There are also other ways to optimize. You are actually storing a hash
for the semaphores, which is allocating a bunch of little blocks anyway.
I think the whole structure can be made much faster with some better
design ideas.

Very true. The connection pool was basically implemented without
performance in mind so far, originally just to have something working,
and was then never being looked at again, except for a PR where the
semaphore was added. I've also overlooked it during the big vibe-core
redesign.

What do you mean by semaphores, though? There is only one of those
per pool and there shouldn't be any small internal allocations going on.

But the AA that is currently used for the reference counts is another
obvious target, although that fortunately shouldn't be a big issue once
the connection count stabilizes, thanks to the open addressing of the
improved AAs. Still not necessary.

Am 28.06.2017 um 00:51 schrieb Steven Schveighoffer:

But the AA that is currently used for the reference counts is another
obvious target, although that fortunately shouldn't be a big issue once
the connection count stabilizes, thanks to the open addressing of the
improved AAs. Still not necessary.

The AA still allocates a block for each element. But you are right it should stabilize at some point.

Hm, I see. I'll have to take a look at the implementation. I was under
the impression that everything is stored in a single flat over-allocated
array (of int in this case).