<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/haproxy.git/include/haproxy/queue-t.h, branch master</title>
<subtitle>github.com: haproxy/haproxy.git
</subtitle>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/'/>
<entry>
<title>CLEANUP: tree-wide: only include ebtree-t from type files</title>
<updated>2021-10-06T23:41:14+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-10-06T16:31:48+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=1db546eecd3982ffc1ea92c2f542a3b01ce43137'/>
<id>1db546eecd3982ffc1ea92c2f542a3b01ce43137</id>
<content type='text'>
No need to include the full tree management code, type files only
need the definitions. Doing so reduces the whole code size by around
3.6% and the build time is down to just 6s.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
No need to include the full tree management code, type files only
need the definitions. Doing so reduces the whole code size by around
3.6% and the build time is down to just 6s.
</pre>
</div>
</content>
</entry>
<entry>
<title>BUG/MAJOR: queue: better protect a pendconn being picked from the proxy</title>
<updated>2021-08-31T16:37:13+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-08-31T15:21:39+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=87154e301043e895dcac1121d779d8879796c7c7'/>
<id>87154e301043e895dcac1121d779d8879796c7c7</id>
<content type='text'>
The locking in the dequeuing process was significantly improved by commit
49667c14b ("MEDIUM: queue: take the proxy lock only during the px queue
accesses") in that it tries hard to limit the time during which the
proxy's queue lock is held to the strict minimum. Unfortunately it's not
enough anymore, because we take up the task and manipulate a few pendconn
elements after releasing the proxy's lock (while we're under the server's
lock) but the task will not necessarily hold the server lock since it may
not have successfully found one (e.g. timeout in the backend queue). As
such, stream_free() calling pendconn_free() may release the pendconn
immediately after the proxy's lock is released while the other thread
currently proceeding with the dequeuing tries to wake up the owner's
task and dies in task_wakeup().

One solution consists in releasing le proxy's lock later. But tests have
shown that we'd have to sacrifice a significant share of the performance
gained with the patch above (roughly a 20% loss).

This patch takes another approach. It adds a "del_lock" to each pendconn
struct, that allows to keep it referenced while the proxy's lock is being
released. It's mostly a serialization lock like a refcount, just to maintain
the pendconn alive till the task_wakeup() call is complete. This way we can
continue to release the proxy's lock early while keeping this one. It had
to be added to the few points where we're about to free a pendconn, namely
in pendconn_dequeue() and pendconn_unlink(). This way we continue to
release the proxy's lock very early and there is no performance degradation.

This lock may only be held under the queue's lock to prevent lock
inversion.

No backport is needed since the patch above was merged in 2.5-dev only.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The locking in the dequeuing process was significantly improved by commit
49667c14b ("MEDIUM: queue: take the proxy lock only during the px queue
accesses") in that it tries hard to limit the time during which the
proxy's queue lock is held to the strict minimum. Unfortunately it's not
enough anymore, because we take up the task and manipulate a few pendconn
elements after releasing the proxy's lock (while we're under the server's
lock) but the task will not necessarily hold the server lock since it may
not have successfully found one (e.g. timeout in the backend queue). As
such, stream_free() calling pendconn_free() may release the pendconn
immediately after the proxy's lock is released while the other thread
currently proceeding with the dequeuing tries to wake up the owner's
task and dies in task_wakeup().

One solution consists in releasing le proxy's lock later. But tests have
shown that we'd have to sacrifice a significant share of the performance
gained with the patch above (roughly a 20% loss).

This patch takes another approach. It adds a "del_lock" to each pendconn
struct, that allows to keep it referenced while the proxy's lock is being
released. It's mostly a serialization lock like a refcount, just to maintain
the pendconn alive till the task_wakeup() call is complete. This way we can
continue to release the proxy's lock early while keeping this one. It had
to be added to the few points where we're about to free a pendconn, namely
in pendconn_dequeue() and pendconn_unlink(). This way we continue to
release the proxy's lock very early and there is no performance degradation.

This lock may only be held under the queue's lock to prevent lock
inversion.

No backport is needed since the patch above was merged in 2.5-dev only.
</pre>
</div>
</content>
</entry>
<entry>
<title>MINOR: queue: remove the px/srv fields from pendconn</title>
<updated>2021-06-24T08:52:31+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-23T14:43:45+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=51c63f0f0a9a629f298bfaeda158599707e562f4'/>
<id>51c63f0f0a9a629f298bfaeda158599707e562f4</id>
<content type='text'>
Now we directly use p-&gt;queue to get to the queue, which is much more
straightforward. The performance on 100 servers and 16 threads
increased from 560k to 574k RPS, or 2.5%.

A lot more simplifications are possible, but the minimum was done at
this point.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Now we directly use p-&gt;queue to get to the queue, which is much more
straightforward. The performance on 100 servers and 16 threads
increased from 560k to 574k RPS, or 2.5%.

A lot more simplifications are possible, but the minimum was done at
this point.
</pre>
</div>
</content>
</entry>
<entry>
<title>MINOR: queue: store a pointer to the queue into the pendconn</title>
<updated>2021-06-24T08:52:31+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-23T14:33:52+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=8429097c61690844caacd90ff336be8b8b4f0fd2'/>
<id>8429097c61690844caacd90ff336be8b8b4f0fd2</id>
<content type='text'>
By following the queue pointer in the pendconn it will now be possible
to always retrieve the elements (index, srv, px, etc).
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
By following the queue pointer in the pendconn it will now be possible
to always retrieve the elements (index, srv, px, etc).
</pre>
</div>
</content>
</entry>
<entry>
<title>MINOR: queue: add a pointer to the server and the proxy in the queue</title>
<updated>2021-06-24T08:52:31+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-23T14:11:02+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=cdc83e01922fa9bf4a9442dbaca9a2fd9e881c12'/>
<id>cdc83e01922fa9bf4a9442dbaca9a2fd9e881c12</id>
<content type='text'>
A queue is specific to a server or a proxy, so we don't need to place
this distinction inside all pendconns, it can be in the queue itself.
This commit adds the relevant fields "px" and "sv" into the struct
queue, and initializes them accordingly.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
A queue is specific to a server or a proxy, so we don't need to place
this distinction inside all pendconns, it can be in the queue itself.
This commit adds the relevant fields "px" and "sv" into the struct
queue, and initializes them accordingly.
</pre>
</div>
</content>
</entry>
<entry>
<title>MEDIUM: queue: use a dedicated lock for the queues (v2)</title>
<updated>2021-06-24T08:52:31+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-18T07:45:27+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=16fbdda3c33703702462a46805a88673e7f6ca51'/>
<id>16fbdda3c33703702462a46805a88673e7f6ca51</id>
<content type='text'>
Till now whenever a server or proxy's queue was touched, this server
or proxy's lock was taken. Not only this requires distinct code paths,
but it also causes unnecessary contention with other uses of these locks.

This patch adds a lock inside the "queue" structure that will be used
the same way by the server and the proxy queuing code. The server used
to use a spinlock and the proxy an rwlock, though the queue only used
it for locked writes. This new version uses a spinlock since we don't
need the read lock part here. Tests have not shown any benefit nor cost
in using this one versus the rwlock so we could change later if needed.

The lower contention on the locks increases the performance from 362k
to 374k req/s on 16 threads with 20 servers and leastconn. The gain
with roundrobin even increases by 9%.

This is tagged medium because the lock is changed, but no other part of
the code touches the queues, with nor without locking, so this should
remain invisible.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Till now whenever a server or proxy's queue was touched, this server
or proxy's lock was taken. Not only this requires distinct code paths,
but it also causes unnecessary contention with other uses of these locks.

This patch adds a lock inside the "queue" structure that will be used
the same way by the server and the proxy queuing code. The server used
to use a spinlock and the proxy an rwlock, though the queue only used
it for locked writes. This new version uses a spinlock since we don't
need the read lock part here. Tests have not shown any benefit nor cost
in using this one versus the rwlock so we could change later if needed.

The lower contention on the locks increases the performance from 362k
to 374k req/s on 16 threads with 20 servers and leastconn. The gain
with roundrobin even increases by 9%.

This is tagged medium because the lock is changed, but no other part of
the code touches the queues, with nor without locking, so this should
remain invisible.
</pre>
</div>
</content>
</entry>
<entry>
<title>Revert "MEDIUM: queue: use a dedicated lock for the queues"</title>
<updated>2021-06-24T05:26:28+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-24T05:26:28+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=3f70fb9ea2d8bdcc6f6fa8190b604f3637349bc7'/>
<id>3f70fb9ea2d8bdcc6f6fa8190b604f3637349bc7</id>
<content type='text'>
This reverts commit fcb8bf8650ec6b5614d1b88db54f1200ebd96cbd.

The recent changes since 5304669e1 MEDIUM: queue: make
pendconn_process_next_strm() only return the pendconn opened a tiny race
condition between stream_free() and process_srv_queue(), as the pendconn
is accessed outside of the lock, possibly while it's being freed. A
different approach is required.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This reverts commit fcb8bf8650ec6b5614d1b88db54f1200ebd96cbd.

The recent changes since 5304669e1 MEDIUM: queue: make
pendconn_process_next_strm() only return the pendconn opened a tiny race
condition between stream_free() and process_srv_queue(), as the pendconn
is accessed outside of the lock, possibly while it's being freed. A
different approach is required.
</pre>
</div>
</content>
</entry>
<entry>
<title>MEDIUM: queue: use a dedicated lock for the queues</title>
<updated>2021-06-22T16:43:56+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-18T07:45:27+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=fcb8bf8650ec6b5614d1b88db54f1200ebd96cbd'/>
<id>fcb8bf8650ec6b5614d1b88db54f1200ebd96cbd</id>
<content type='text'>
Till now whenever a server or proxy's queue was touched, this server
or proxy's lock was taken. Not only this requires distinct code paths,
but it also causes unnecessary contention with other uses of these locks.

This patch adds a lock inside the "queue" structure that will be used
the same way by the server and the proxy queuing code. The server used
to use a spinlock and the proxy an rwlock, though the queue only used
it for locked writes. This new version uses a spinlock since we don't
need the read lock part here. Tests have not shown any benefit nor cost
in using this one versus the rwlock so we could change later if needed.

The lower contention on the locks increases the performance from 491k
to 507k req/s on 16 threads with 20 servers and leastconn. The gain
with roundrobin even increases by 6%.

The performance profile changes from this:
  13.03%  haproxy             [.] fwlc_srv_reposition
   8.08%  haproxy             [.] fwlc_get_next_server
   3.62%  haproxy             [.] process_srv_queue
   1.78%  haproxy             [.] pendconn_dequeue
   1.74%  haproxy             [.] pendconn_add

to this:
  11.95%  haproxy             [.] fwlc_srv_reposition
   7.57%  haproxy             [.] fwlc_get_next_server
   3.51%  haproxy             [.] process_srv_queue
   1.74%  haproxy             [.] pendconn_dequeue
   1.70%  haproxy             [.] pendconn_add

At this point the differences are mostly measurement noise.

This is tagged medium because the lock is changed, but no other part of
the code touches the queues, with nor without locking, so this should
remain invisible.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Till now whenever a server or proxy's queue was touched, this server
or proxy's lock was taken. Not only this requires distinct code paths,
but it also causes unnecessary contention with other uses of these locks.

This patch adds a lock inside the "queue" structure that will be used
the same way by the server and the proxy queuing code. The server used
to use a spinlock and the proxy an rwlock, though the queue only used
it for locked writes. This new version uses a spinlock since we don't
need the read lock part here. Tests have not shown any benefit nor cost
in using this one versus the rwlock so we could change later if needed.

The lower contention on the locks increases the performance from 491k
to 507k req/s on 16 threads with 20 servers and leastconn. The gain
with roundrobin even increases by 6%.

The performance profile changes from this:
  13.03%  haproxy             [.] fwlc_srv_reposition
   8.08%  haproxy             [.] fwlc_get_next_server
   3.62%  haproxy             [.] process_srv_queue
   1.78%  haproxy             [.] pendconn_dequeue
   1.74%  haproxy             [.] pendconn_add

to this:
  11.95%  haproxy             [.] fwlc_srv_reposition
   7.57%  haproxy             [.] fwlc_get_next_server
   3.51%  haproxy             [.] process_srv_queue
   1.74%  haproxy             [.] pendconn_dequeue
   1.70%  haproxy             [.] pendconn_add

At this point the differences are mostly measurement noise.

This is tagged medium because the lock is changed, but no other part of
the code touches the queues, with nor without locking, so this should
remain invisible.
</pre>
</div>
</content>
</entry>
<entry>
<title>MINOR: queue: create a new structure type "queue"</title>
<updated>2021-06-22T16:43:14+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2021-06-18T07:21:22+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=eea3817a47586eb27340f37bd77be155c679f7a0'/>
<id>eea3817a47586eb27340f37bd77be155c679f7a0</id>
<content type='text'>
This structure will be common to proxies and servers and will contain
everything needed to handle their respective queues. For now it's only
a tree head, a length and an index.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This structure will be common to proxies and servers and will contain
everything needed to handle their respective queues. For now it's only
a tree head, a length and an index.
</pre>
</div>
</content>
</entry>
<entry>
<title>REORG: include: move queue.h to haproxy/queue{,-t}.h</title>
<updated>2020-06-11T08:18:58+00:00</updated>
<author>
<name>Willy Tarreau</name>
<email>w@1wt.eu</email>
</author>
<published>2020-06-04T20:59:39+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/haproxy.git/commit/?id=a55c45470fa0e107c9196d5fc1098504a9b9b1f6'/>
<id>a55c45470fa0e107c9196d5fc1098504a9b9b1f6</id>
<content type='text'>
Nothing outstanding here. A number of call places were not justified and
removed.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Nothing outstanding here. A number of call places were not justified and
removed.
</pre>
</div>
</content>
</entry>
</feed>
