Issue 5852 = Remove turbo mode from the code base #5893
Conversation
ldap/servers/slapd/connection.c
Outdated
| @@ -1311,7 +1262,7 @@ connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, int * | |||
| if (SLAPD_PR_WOULD_BLOCK_ERROR(err) || | |||
| SLAPD_SYSTEM_WOULD_BLOCK_ERROR(syserr)) { | |||
| struct PRPollDesc pr_pd; | |||
| PRIntervalTime timeout = PR_MillisecondsToInterval(CONN_TURBO_TIMEOUT_INTERVAL); | |||
| PRIntervalTime timeout = PR_MillisecondsToInterval(100); | |||
There was a problem hiding this comment.
Probably should be a constant for future tweaking.
|
I had turbo mode turned off in dse.ldif. After I updated packages, my instance refused to start: I think we should not make this an error, but rather WARN or INFO, and not prevent DS from starting. |
ldap/servers/slapd/connection.c
Outdated
| @@ -1217,7 +1168,7 @@ connection_read_operation(Connection *conn, Operation *op, ber_tag_t *tag, int * | |||
|
|
|||
| /* If we still haven't seen a complete PDU, read from the network */ | |||
| while (*tag == LBER_DEFAULT) { | |||
| int32_t ioblocktimeout_waits = conn->c_ioblocktimeout / CONN_TURBO_TIMEOUT_INTERVAL; | |||
| int32_t ioblocktimeout_waits = conn->c_ioblocktimeout; | |||
There was a problem hiding this comment.
My understanding is that ioblocktimeout_waits is a count of the number of wait, not a duration. So we should set it to the number of time we 'waits_done'.
This is why it divides ioblocktimeout by CONN_TURBO_TIMEOUT_INTERVAL and sleep for CONN_TURBO_TIMEOUT_INTERVAL.
| @@ -1731,52 +1531,11 @@ connection_threadmain(void *arg) | |||
| maxthreads = conn->c_max_threads_per_conn; | |||
| more_data = 0; | |||
| ret = connection_read_operation(conn, op, &tag, &more_data); | |||
| if ((ret == CONN_DONE) || (ret == CONN_TIMEDOUT)) { | |||
There was a problem hiding this comment.
This block is just logging debug info regarding the connection. It logs info about turbo mode, that should be removed, but the rest of the message looks good to me.
| * c_idlesince is set in handle_pr_read_ready - since we | ||
| * are bypassing both of those, we set idlesince here | ||
| */ | ||
| conn->c_idlesince = curtime; |
There was a problem hiding this comment.
Why removing update of idlesince ? There are more data on the socket so it looks to me we can update idlesince.
Description: Turbo mode is a feature that improves the search performance for a low number of highly active connections. Since its introduction the deployment model has changed and this feature is no longer required. Turbo mode adds some complexity to the code so removal will simplify connection code. Fix description: Following investigation into the pros/cons of turbo mode it has been decided that this feature should be removed from the codebase. Relates: 389ds#5852 Reviewed by: ?
|
The nsslapd-enable-turbo-mode attribute will remain in the code for a short while, the user will be able to get the attr value and will receive an info message when trying to replace its value. In the initial commit, I return error code 1 (operations error) when the user tries to replace the attr value, which will issue an info message to the user. But, in the corner case where the user has upgraded DS, instance restart explodes while loading dse.ldif (#5893 (comment)) So instead, I return success when the user tries to replace the attr value and I have added the attr type to reject_attr_type() in configdse.c. This way I can issue a message to the user when a replace is attempted and maintain dse compatibility. What do you think ? |
|
A performance issue was found with the replication total init testcase (see #5761). Additional tests during that investigation, focus on turbo mode impact. It is looking turbo mode was beneficial. It would be interesting to verify turbo-mode removal on total init |
I agree, following our discussion yesterday it seems like turbo mode could play a part, well for a total init anyway. I will open a ticket to investigate this. |
|
|
Another thing to clarify is that bind/accept slowness when turbo mode is 'on'. (see solution 4292701). ATM it is not clear to me why it is slower, possibly the consequence a long computation to switch on/off turbo mode. |
Description: Turbo mode is a feature that improves the search performance for a low number of highly active connections. Since its introduction the deployment model has changed and this feature is no longer required. Turbo mode adds some complexity to the code so removal will simplify connection code.
Fix description: Following investigation into the pros/cons of turbo mode it has been decided that this feature should be removed from the codebase.
Relates: #5852
Reviewed by: ?