Add scalable FIFO support to LogThreadedSource based sources#3969
Merged
MrAnno merged 22 commits intoJan 6, 2023
Conversation
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
240c477 to
832d2ba
Compare
Contributor
|
Build FAILURE |
1 similar comment
Contributor
|
Build FAILURE |
bazsi
changed the title
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
24b2dce to
2b9d7c8
Compare
Contributor
|
Build FAILURE |
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
2 times, most recently
from
d165e03 to
c898e56
Compare
Contributor
|
Build SUCCESS |
bazsi
changed the title
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
c898e56 to
491709b
Compare
Contributor
|
Build SUCCESS |
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
2 times, most recently
from
81bba33 to
a7bebaf
Compare
Contributor
|
Build SUCCESS |
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
a7bebaf to
d2619a7
Compare
Contributor
|
Build SUCCESS |
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
d2619a7 to
6b2f0fc
Compare
Collaborator
Author
|
Rebased against current master. |
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
6b2f0fc to
3a2466d
Compare
Contributor
|
Build FAILURE |
Collaborator
Author
|
@kira-syslogng retest this please; |
Contributor
|
@kira-syslogng retest this please; |
Contributor
|
Build FAILURE |
Contributor
|
@kira-syslogng retest this please; |
MrAnno
suggested changes
Jan 2, 2023
Collaborator
MrAnno
left a comment
MrAnno
left a comment
There was a problem hiding this comment.
I've marked my important note with [change-request].
|
|
||
| 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++) |
Collaborator
There was a problem hiding this comment.
This variable is undeclared.
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
3a2466d to
ad4596a
Compare
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
ad4596a to
fc585b7
Compare
Collaborator
Author
|
rebased and squashed. |
Contributor
|
Build FAILURE |
Collaborator
|
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 |
Collaborator
Author
|
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>
bazsi
force-pushed
the
logthrsource-scalable-fifo-support
branch
from
fc585b7 to
ddf3758
Compare
Contributor
|
Build FAILURE |
MrAnno
approved these changes
Jan 6, 2023
Collaborator
|
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.