Hi,
attached is a patch that I think is correct and it did solve a bug I was seeing, but I'm not sure if it's the right thing to do since the code was stable since 2009..
What I was seeing was a scenario where an LDAP server was listening on port 389, so we were able to connect there with a socket, but then all searches timed out during rootDSE discovery.. What happened in the sdap_id_ops.c code was that we hit this part:
810 switch (retval) { 811 case EIO: 812 ---> case ETIMEDOUT: 813 /* this currently the only possible communication error after connection is established */ 814 communication_error = true; 815 break; 816 817 default: 818 communication_error = false; 819 break; 820 } 821
And then we went here, because the connection was already established:
822 if (communication_error && current_conn != 0 823 && current_conn == op->conn_cache->cached_connection) { 824 /* do not reuse failed connection */ 825 op->conn_cache->cached_connection = NULL; 826 827 DEBUG(SSSDBG_FUNC_DATA, 828 "communication error on cached connection, moving to next server\n"); 829 ----> be_fo_try_next_server(op->conn_cache->id_conn->id_ctx->be, 830 op->conn_cache->id_conn->service->name); 831 }
But I admit I don't understand why does be_fo_try_next_server() set the port status to NEUTRAL. That caused the connection code to run again, hit the same timeout issue and then cycle again and again..
Can anyone parse from the code why do we set the port to neutral instead of not_working in be_fo_try_next_server() ?
On 05/10/2016 04:33 PM, Jakub Hrozek wrote:
Hi,
attached is a patch that I think is correct and it did solve a bug I was seeing, but I'm not sure if it's the right thing to do since the code was stable since 2009..
What I was seeing was a scenario where an LDAP server was listening on port 389, so we were able to connect there with a socket, but then all searches timed out during rootDSE discovery.. What happened in the sdap_id_ops.c code was that we hit this part:
810 switch (retval) { 811 case EIO: 812 ---> case ETIMEDOUT: 813 /* this currently the only possible communication error after connection is established */ 814 communication_error = true; 815 break; 816 817 default: 818 communication_error = false; 819 break; 820 } 821
And then we went here, because the connection was already established:
822 if (communication_error && current_conn != 0 823 && current_conn == op->conn_cache->cached_connection) { 824 /* do not reuse failed connection */ 825 op->conn_cache->cached_connection = NULL; 826 827 DEBUG(SSSDBG_FUNC_DATA, 828 "communication error on cached connection, moving to next server\n"); 829 ----> be_fo_try_next_server(op->conn_cache->id_conn->id_ctx->be, 830 op->conn_cache->id_conn->service->name); 831 }
But I admit I don't understand why does be_fo_try_next_server() set the port status to NEUTRAL. That caused the connection code to run again, hit the same timeout issue and then cycle again and again..
Can anyone parse from the code why do we set the port to neutral instead of not_working in be_fo_try_next_server() ?
Sorry for the delay, I finally managed to look at the code. PORT_NOT_WORKING seems to be the correct value here. I think this is just another patch that raises the need to refactor our failover code since none of us really understand it. :-)
Ack.
On Fri, Jul 08, 2016 at 10:53:12AM +0200, Pavel Březina wrote:
On 05/10/2016 04:33 PM, Jakub Hrozek wrote:
Hi,
attached is a patch that I think is correct and it did solve a bug I was seeing, but I'm not sure if it's the right thing to do since the code was stable since 2009..
What I was seeing was a scenario where an LDAP server was listening on port 389, so we were able to connect there with a socket, but then all searches timed out during rootDSE discovery.. What happened in the sdap_id_ops.c code was that we hit this part:
810 switch (retval) { 811 case EIO: 812 ---> case ETIMEDOUT: 813 /* this currently the only possible communication error after connection is established */ 814 communication_error = true; 815 break; 816 817 default: 818 communication_error = false; 819 break; 820 } 821
And then we went here, because the connection was already established:
822 if (communication_error && current_conn != 0 823 && current_conn == op->conn_cache->cached_connection) { 824 /* do not reuse failed connection */ 825 op->conn_cache->cached_connection = NULL; 826 827 DEBUG(SSSDBG_FUNC_DATA, 828 "communication error on cached connection, moving to next server\n"); 829 ----> be_fo_try_next_server(op->conn_cache->id_conn->id_ctx->be, 830 op->conn_cache->id_conn->service->name); 831 }
But I admit I don't understand why does be_fo_try_next_server() set the port status to NEUTRAL. That caused the connection code to run again, hit the same timeout issue and then cycle again and again..
Can anyone parse from the code why do we set the port to neutral instead of not_working in be_fo_try_next_server() ?
Sorry for the delay, I finally managed to look at the code. PORT_NOT_WORKING seems to be the correct value here. I think this is just another patch that raises the need to refactor our failover code since none of us really understand it. :-)
Ack.
* master: c420ce830ac0b0b288a2a887ec2cfce5c748018c
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Pavel Březina