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
Add scalable FIFO support to LogThreadedSource based sources #3969
Add scalable FIFO support to LogThreadedSource based sources #3969
Conversation
240c477
to
832d2ba
Compare
Build FAILURE |
1 similar comment
Build FAILURE |
24b2dce
to
2b9d7c8
Compare
Build FAILURE |
d165e03
to
c898e56
Compare
Build SUCCESS |
c898e56
to
491709b
Compare
Build SUCCESS |
81bba33
to
a7bebaf
Compare
Build SUCCESS |
a7bebaf
to
d2619a7
Compare
Build SUCCESS |
d2619a7
to
6b2f0fc
Compare
Rebased against current master. |
6b2f0fc
to
3a2466d
Compare
Build FAILURE |
@kira-syslogng retest this please; |
@kira-syslogng retest this please; |
Build FAILURE |
@kira-syslogng retest this please; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've marked my important note with [change-request]
.
lib/logqueue-fifo.c
Outdated
@@ -673,7 +678,8 @@ log_queue_fifo_new(gint log_fifo_size, const gchar *persist_name) | |||
|
|||
self->super.free_fn = log_queue_fifo_free; | |||
|
|||
for (i = 0; i < log_queue_max_threads; i++) | |||
self->num_input_queues = log_queue_max_threads; | |||
for (i = 0; i < num_input_queues; i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is undeclared.
3a2466d
to
ad4596a
Compare
ad4596a
to
fc585b7
Compare
rebased and squashed. |
Build FAILURE |
There is still a fixup commit in the history. It seems the commit check is broken, it shows success incorrectly. I'm fixing it: #4270 |
Sorry, I needed to move the bank port patch earlier as autosquash failed at
first. Let me do that again.
…On Thu, Jan 5, 2023, 10:39 László Várady ***@***.***> wrote:
There is still a fixup commit in the history.
It seems the commit check is broken, it shows success incorrectly. I'm
fixing it.
—
Reply to this email directly, view it on GitHub
<#3969 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFOK5T7QKND65XVXWJXUALWQ2JFNANCNFSM5SBGXIIQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This patch removes the 64 thread limitation from _allocate_thread_id and increases the current maximum to 256. Allocations that depend on per-thread state currently only use the actual number of threads allowed by the --worker-threads command line option (which defaults to the number of cores in the system), so increasing this limit does not cause an immediate increase in memory usage. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
This naming matches the existing naming convention of LogPipe method names, also I intend to introduce a pre_config_init() hook as well. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
…reads Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
… each message Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Instead of relying on a single global log_queue_max_threads variable, save it at constructor time. Also, handle the case where we get a thread_id that is larger than this saved value, instead of failing an assert. This patch is a preparatory step to allow threaded sources to use the scalable input path for a LogQueue, because if we want to support that we need to make the thread_id space configuration dependent. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
… max_threads value Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Instead of a layering violation (cfg calling into mainloop-worker) and to allow registering thread IDs from non-LogPipe instances (e.g. the mainloop-io-worker layer). Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
The "thread_id" value had dual uses: 1) at one point it was 1 based, so that 0 meant uninitialized state 2) in other cases it was 0 based, so we could use it as an array index This patch makes the two uses distinct. The "id" term is only referred to the 1 based value, 0 meaning uninitialized. In all other cases we are referring to "thread_index" to better reflect that this is a 0 based value. In the latter case, -1 means uninitialized. Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
fc585b7
to
ddf3758
Compare
Build FAILURE |
The macOS runner seems to have been updated and now it contains a broken net-snmp pc file. I'm working on a fix. |
This is a spin-off from #3966 which merges the thread ID space for worker threads and associates a thread_id even with
threads spawned by LogThreadedSourceDriver. This will enable the scalable input processing in LogQueueFifo and makes a few things simpler.
It also increases the number of supported threads to 256 which is becoming a simple compile-time constant, instead of a constraint imposed by the underlying data structure (the previous limit of 64 being the number of bits in a guint64).
While #3966 still depends on an ivykis change (buytenh/ivykis#24) but I don't want to stall the commits that make sense even without the ivykis dependency.