Skip to content
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

Backport of accept thread to 389-ds-base-1.4.3 #4870

Merged
merged 2 commits into from Aug 16, 2021

Conversation

jchapma
Copy link
Contributor

@jchapma jchapma commented Aug 11, 2021

The initial PR (#4865) for this backport auto closed when I pushed a second commit...

Issue 4506 - RFE - connection accept thread

Bug Description: Previously we accepted connections and
selected for new work in the same event loop. This could
cause connection table polling to delay accepts, and
accepts to delay connection activity from being ready.

Fix Description: This seperates those functions allowing
accept to occur in parallel to our normal work.

fixes: 389ds#4506

Author: William Brown <william@blackhats.net.au>

Review by: @mreynolds389 @progier389 (Thanks!)
Bug Description: during review it was requested that a piece
of code be changed which seemed quite innocent. The code was
moved but the logic around the code wasn't considered
causing the fd array for the accept thread to be allocated with
a size of zero, causing the values to be lost.

Fix Description: Move the allocation to the correct location.

fixes: 389ds#4506

Author: William Brown <william@blackhats.net.au>

Review by: @mreynolds389 @droideck
@Firstyear
Copy link
Contributor

Good catch, my previous ack stands :)

Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've run a few test suites and everything seems fine. You have my ack too!

@jchapma jchapma merged commit c7e7259 into 389ds:389-ds-base-1.4.3 Aug 16, 2021
@jchapma jchapma deleted the 389-ds-base-1.4.3 branch March 28, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants