GUACAMOLE-2180: Improve SFTP upload performance#629
Open
alailsonko wants to merge 2 commits intoapache:mainfrom
Open
GUACAMOLE-2180: Improve SFTP upload performance#629alailsonko wants to merge 2 commits intoapache:mainfrom
alailsonko wants to merge 2 commits intoapache:mainfrom
Conversation
alailsonko
changed the title
necouchman
requested changes
Jan 25, 2026
Contributor
necouchman
left a comment
necouchman
left a comment
There was a problem hiding this comment.
Overall I think it looks okay - just needs some updated documentation.
| */ | ||
| #define GUAC_COMMON_SSH_SFTP_UPLOAD_BUFFER_SIZE (256 * 1024) | ||
|
|
||
| typedef struct guac_common_ssh_sftp_upload_state { |
Contributor
There was a problem hiding this comment.
Please provide overall documentation for the data structure.
Comment on lines
+72
to
+97
| static void guac_common_ssh_sftp_upload_flush(guac_user* user, | ||
| guac_common_ssh_sftp_upload_state* state) { | ||
|
|
||
| if (state->error || state->buffered_length <= 0 || state->file == NULL) | ||
| return; | ||
|
|
||
| int offset = 0; | ||
| while (offset < state->buffered_length) { | ||
| ssize_t written = libssh2_sftp_write(state->file, | ||
| state->buffer + offset, state->buffered_length - offset); | ||
| if (written <= 0) { | ||
| guac_user_log(user, GUAC_LOG_INFO, | ||
| "Buffered SFTP write failed after %d bytes (attempt returned %zd)", | ||
| offset, written); | ||
| state->error = 1; | ||
| break; | ||
| } | ||
| offset += (int)written; | ||
| } | ||
|
|
||
| if (!state->error) | ||
| guac_user_log(user, GUAC_LOG_DEBUG, | ||
| "Flushed %d buffered bytes to remote file", state->buffered_length); | ||
|
|
||
| state->buffered_length = 0; | ||
| } |
Contributor
There was a problem hiding this comment.
Some documentation throughout this function would be nice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request significantly improves the efficiency and reliability of SFTP file uploads by introducing buffered uploads and robust error handling. Instead of writing each incoming data blob directly to the remote file, uploads are now buffered in memory and flushed in larger chunks, reducing the number of SFTP write operations and handling partial writes more gracefully. The code also introduces a per-upload state structure to track progress and errors, and ensures proper cleanup at the end of each upload.
Buffered SFTP uploads
guac_common_ssh_sftp_upload_state) to manage buffered data, the SFTP file handle, and error status for each upload stream.Related Jira Item