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

make sysrepo connection timeout switchable #1099

Merged
merged 3 commits into from
Apr 24, 2018
Merged

make sysrepo connection timeout switchable #1099

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1878d0b
make sysrepo connection timeout configurable
frimpler Apr 17, 2018
672e018
update to PR#1099: use SR_REQUEST_TIMEOUT/SR_LONG_REQUEST_TIMEOUT ins…
frimpler Apr 20, 2018
beb8692
PR1099: incorporate suggestions from Radek
frimpler Apr 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,10 @@ set(STORE_CONFIG_CHANGE_NOTIF 1 CACHE BOOL

# timeouts
set(REQUEST_TIMEOUT 4 CACHE INTEGER
"Timeout (in seconds) for standard Sysrepo API requests.")
"Timeout (in seconds) for standard Sysrepo API requests. Set to 0 for no timeout.")

set(LONG_REQUEST_TIMEOUT 15 CACHE INTEGER
"Timeout (in seconds) for Sysrepo API requests that can take longer than standard requests (commit, copy-config, rpc, action).")
"Timeout (in seconds) for Sysrepo API requests that can take longer than standard requests (commit, copy-config, rpc, action). Set to 0 for no timeout.")

set(COMMIT_VERIFY_TIMEOUT 10 CACHE INTEGER
"Timeout (in seconds) that a commit request can wait for answer from commit verifiers and change notification subscribers.")
Expand All @@ -194,6 +194,10 @@ set(NOTIF_AGE_TIMEOUT 60 CACHE INTEGER
set(NOTIF_TIME_WINDOW 10 CACHE INTEGER
"Time window (in minutes) for notifications to be grouped into one data file (larger window produces larger data files).")

if(REQUEST_TIMEOUT EQUAL "0" AND NOT LONG_REQUEST_TIMEOUT EQUAL "0")
message(FATAL_ERROR "LONG_REQUEST_TIMEOUT must be 0 when REQUEST_TIMEOUT is 0")
Copy link
Contributor

Choose a reason for hiding this comment

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

in this case I would set LONG_REQUEST_TIMEOUT automatically (maybe with some message to get know what it does), on the other hand, I would like to have a test (and error) for the case REQUEST_TIMEOUT > LONG_REQUEST_TIMEOUT.

endif(REQUEST_TIMEOUT EQUAL "0" AND NOT LONG_REQUEST_TIMEOUT EQUAL "0")

# add subdirectories
add_subdirectory(src)

Expand Down
4 changes: 2 additions & 2 deletions INSTALL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ There are several timeouts that can be configured via CMake variables:

CMake variable | Default value | Description
--------------------------- | ------------- | -----------
`REQUEST_TIMEOUT` | 3 sec | Timeout (in seconds) for standard Sysrepo API requests.
`LONG_REQUEST_TIMEOUT` | 15 sec | Timeout (in seconds) for Sysrepo API requests that can take longer than standard requests (commit, copy-config, rpc, action).
`REQUEST_TIMEOUT` | 3 sec | Timeout (in seconds) for standard Sysrepo API requests. Set to 0 to disable request timeouts.
`LONG_REQUEST_TIMEOUT` | 15 sec | Timeout (in seconds) for Sysrepo API requests that can take longer than standard requests (commit, copy-config, rpc, action). Set to 0 to disable timeouts.
`COMMIT_VERIFY_TIMEOUT` | 10 sec | Timeout (in seconds) that a commit request can wait for answer from commit verifiers and change notification subscribers.
`OPER_DATA_PROVIDE_TIMEOUT` | 2 sec | Timeout (in seconds) that a request can wait for operational data from data providers.
`NOTIF_AGE_TIMEOUT` | 60 min | Timeout (in minutes) after which stored notifications will be aged out and erased from notification store.
Expand Down
26 changes: 15 additions & 11 deletions src/clientlib/cl_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,15 @@ cl_socket_connect(sr_conn_ctx_t *conn_ctx, const char *socket_path)
}

/* set timeout for receive operation */
tv.tv_sec = SR_REQUEST_TIMEOUT;
tv.tv_usec = 0;
rc = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv));
if (-1 == rc) {
SR_LOG_ERR("Unable to set timeout for socket operations on socket=%s: %s", socket_path, sr_strerror_safe(errno));
close(fd);
return SR_ERR_DISCONNECT;
if(SR_REQUEST_TIMEOUT > 0) {
tv.tv_sec = SR_REQUEST_TIMEOUT;
tv.tv_usec = 0;
rc = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv));
if (-1 == rc) {
SR_LOG_ERR("Unable to set timeout for socket operations on socket=%s: %s", socket_path, sr_strerror_safe(errno));
close(fd);
return SR_ERR_DISCONNECT;
}
}

conn_ctx->fd = fd;
Expand Down Expand Up @@ -493,8 +495,9 @@ cl_request_process(sr_session_ctx_t *session, Sr__Msg *msg_req, Sr__Msg **msg_re

pthread_mutex_lock(&session->conn_ctx->lock);
/* some operation may take more time, raise the timeout */
if (SR__OPERATION__COMMIT == expected_response_op || SR__OPERATION__COPY_CONFIG == expected_response_op ||
SR__OPERATION__RPC == expected_response_op || SR__OPERATION__ACTION == expected_response_op) {
if ((SR__OPERATION__COMMIT == expected_response_op || SR__OPERATION__COPY_CONFIG == expected_response_op ||
SR__OPERATION__RPC == expected_response_op || SR__OPERATION__ACTION == expected_response_op) &&
SR_LONG_REQUEST_TIMEOUT > 0) {
tv.tv_sec = SR_LONG_REQUEST_TIMEOUT;
tv.tv_usec = 0;
rc = setsockopt(session->conn_ctx->fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv));
Expand Down Expand Up @@ -524,8 +527,9 @@ cl_request_process(sr_session_ctx_t *session, Sr__Msg *msg_req, Sr__Msg **msg_re
}

/* change socket timeout to the standard value */
if (SR__OPERATION__COMMIT == expected_response_op || SR__OPERATION__COPY_CONFIG == expected_response_op ||
SR__OPERATION__RPC == expected_response_op || SR__OPERATION__ACTION == expected_response_op) {
if ((SR__OPERATION__COMMIT == expected_response_op || SR__OPERATION__COPY_CONFIG == expected_response_op ||
SR__OPERATION__RPC == expected_response_op || SR__OPERATION__ACTION == expected_response_op) &&
SR_LONG_REQUEST_TIMEOUT > 0) {
tv.tv_sec = SR_REQUEST_TIMEOUT;
rc = setsockopt(session->conn_ctx->fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv));
if (-1 == rc) {
Expand Down