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.

Sign up for GitHub

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

Jump to bottom

performance search rate: useless poll on network send callback #4316

Closed
389-ds-bot opened this issue Sep 13, 2020 · 8 comments
Closed

performance search rate: useless poll on network send callback #4316

389-ds-bot opened this issue Sep 13, 2020 · 8 comments
Labels
performance Issue impacts performance priority_high need urgent fix / highly valuable / easy to fix
Milestone
1.4.4

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/51263

Issue Description

DS writes to the socket using a callback function (openldap_write_function). This function tests (poll) the ability to write before sending pdu. This is useless as there is already a loop that taking care of retry/partial write.
This adds an overhead of 'pool' syscall. It improves througput up to 5%.

perf trace -t <worker-tid> -s (taken on top of 51262: nagle off)

With poll+sendto

 ns-slapd (93047), 540445 events, 100.0%

   syscall            calls    total       min       avg       max      stddev
                               (msec)    (msec)    (msec)    (msec)        (%)
   --------------- -------- --------- --------- --------- ---------     ------
   futex             103768   524.042     0.001     0.005     2.971      0.67%
   poll               36964   154.968     0.001     0.004     0.044      0.14%
   sendto             36941   140.178     0.001     0.004     0.025      0.20%
   write              36927   135.035     0.002     0.004     0.026      0.16%
   recvfrom           18495    76.359     0.002     0.004     0.027      0.17%


With sendto

 ns-slapd (117421), 319573 events, 100.0%

   syscall            calls    total       min       avg       max      stddev
                               (msec)    (msec)    (msec)    (msec)        (%)
   --------------- -------- --------- --------- --------- ---------     ------
   futex              83704  1021.638     0.000     0.012   100.113     23.99%
   sendto             30268   245.164     0.002     0.008     0.041      0.22%
   write              30301   123.068     0.002     0.004     0.033      0.23%
   recvfrom           15222    65.530     0.002     0.004     0.034      0.28%
   poll                  40     1.146     0.003     0.029     0.098     12.61%

Package Version and Platform

all versions

Steps to reproduce

perf trace -t -s ==> few poll

Actual results

almost same number of poll than sendto

Expected results

very low number of poll
@389-ds-bot 389-ds-bot added this to the 1.4.4 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-09-10 01:06:41

Are we in control of that poll function? Or is this something inside openldap write we need to change?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2020-09-10 01:06:41

Metadata Update from @Firstyear:

  • Custom field origin adjusted to None
  • Custom field reviewstatus adjusted to None

@389-ds-bot
Copy link
Author

Comment from spichugi (@droideck) at 2020-09-10 12:23:08

Metadata Update from @droideck:

  • Issue close_status updated to: wontfix
  • Issue status updated to: Closed (was: Open)

@389-ds-bot
Copy link
Author

Comment from spichugi (@droideck) at 2020-09-10 12:45:11

Metadata Update from @droideck:

  • Issue status updated to: Open (was: Closed)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2020-09-10 17:36:03

Metadata Update from @mreynolds389:

  • Issue priority set to: normal
  • Issue set to the milestone: 1.4.4

@tbordaz
Copy link
Contributor

tbordaz commented Nov 5, 2020

BZ https://bugzilla.redhat.com/show_bug.cgi?id=1894837

@tbordaz tbordaz mentioned this issue Nov 10, 2020
Merged
tbordaz added a commit to tbordaz/389-ds-base that referenced this issue Nov 12, 2020
@tbordaz
Issue 4316 - performance search rate: useless poll on network send ca…
f6d64c4 
…llback

Bug description:
	When sending back result/entries, DS first poll the connection to check
        it is able to write data on the socket. Then it writes the data.
	The purpose of the poll is to handle ioblocktimeout.
	The problem is that most of the time, the socket will process the write
	without any issue so it is useless to poll before the write.

Fix description:
	The fix is try write first. It polls for ioblocktimeout
        only if the write fails

relates: 389ds#4316

Reviewed by: William Brown (thanks!)

Platforms tested: F31
tbordaz added a commit that referenced this issue Nov 12, 2020
@tbordaz
Issue 4316 - performance search rate: useless poll on network send ca…
99462bb 
…llback (#4424)

Bug description:
	When sending back result/entries, DS first poll the connection to check
        it is able to write data on the socket. Then it writes the data.
	The purpose of the poll is to handle ioblocktimeout.
	The problem is that most of the time, the socket will process the write
	without any issue so it is useless to poll before the write.

Fix description:
	The fix is try write first. It polls for ioblocktimeout
        only if the write fails

relates: #4316

Reviewed by: William Brown (thanks!)

Platforms tested: F31
tbordaz added a commit that referenced this issue Nov 12, 2020
@tbordaz
Issue 4316 - performance search rate: useless poll on network send ca…
0dbdc11 
…llback (#4424)

Bug description:
	When sending back result/entries, DS first poll the connection to check
        it is able to write data on the socket. Then it writes the data.
	The purpose of the poll is to handle ioblocktimeout.
	The problem is that most of the time, the socket will process the write
	without any issue so it is useless to poll before the write.

Fix description:
	The fix is try write first. It polls for ioblocktimeout
        only if the write fails

relates: #4316

Reviewed by: William Brown (thanks!)

Platforms tested: F31
@tbordaz
Copy link
Contributor

tbordaz commented Nov 12, 2020

a924b55..99462bb master
32f30f2..0dbdc11 389-ds-base-1.4.4

@tbordaz tbordaz closed this as completed Nov 12, 2020
@tbordaz tbordaz added the performance Issue impacts performance label Jan 26, 2021
tbordaz added a commit that referenced this issue Jan 19, 2022
@tbordaz
Issue 4316 - performance search rate: useless poll on network send ca…
9e5cdb8 
…llback (#4424)

Bug description:
	When sending back result/entries, DS first poll the connection to check
        it is able to write data on the socket. Then it writes the data.
	The purpose of the poll is to handle ioblocktimeout.
	The problem is that most of the time, the socket will process the write
	without any issue so it is useless to poll before the write.

Fix description:
	The fix is try write first. It polls for ioblocktimeout
        only if the write fails

relates: #4316

Reviewed by: William Brown (thanks!)

Platforms tested: F31
@tbordaz tbordaz added the priority_high need urgent fix / highly valuable / easy to fix label Jan 19, 2022
@tbordaz
Copy link
Contributor

tbordaz commented Jan 19, 2022

e55d750..9e5cdb8 389-ds-base-1.4.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Issue impacts performance priority_high need urgent fix / highly valuable / easy to fix
Projects
None yet
Milestone
1.4.4
Development

No branches or pull requests
2 participants
@tbordaz @389-ds-bot