From 7112f8294c162db536b15f9d527033c9d641e057 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 23 May 2024 11:53:17 -0400 Subject: [PATCH 1/2] Add `SSHMaster::Connection::trySetBufferSize` It is unused in Nix currently, but will be used in Hydra. This reflects what Hydra does in https://github.com/NixOS/hydra/pull/1387. We may probably to use it more widely for better SSH store performance, but this needs to be subject to more testing before we do that. (cherry picked from commit 0d25cc65417647c454e3095650b87bc88351b384) --- src/libstore/ssh.cc | 15 +++++++++++++++ src/libstore/ssh.hh | 12 ++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 116a480ba..f47cfbbec 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -240,4 +240,19 @@ Path SSHMaster::startMaster() #endif +void SSHMaster::Connection::trySetBufferSize(size_t size) +{ +#ifdef F_SETPIPE_SZ + /* This `fcntl` method of doing this takes a positive `int`. Check + and convert accordingly. + + The function overall still takes `size_t` because this is more + portable for a platform-agnostic interface. */ + assert(size <= INT_MAX); + int pipesize = size; + fcntl(in.get(), F_SETPIPE_SZ, pipesize); + fcntl(out.get(), F_SETPIPE_SZ, pipesize); +#endif +} + } diff --git a/src/libstore/ssh.hh b/src/libstore/ssh.hh index 85be704ec..eb05df011 100644 --- a/src/libstore/ssh.hh +++ b/src/libstore/ssh.hh @@ -54,6 +54,18 @@ public: Pid sshPid; #endif AutoCloseFD out, in; + + /** + * Try to set the buffer size in both directions to the + * designated amount, if possible. If not possible, does + * nothing. + * + * Current implementation is to use `fcntl` with `F_SETPIPE_SZ`, + * which is Linux-only. For this implementation, `size` must + * convertable to an `int`. In other words, it must be within + * `[0, INT_MAX]`. + */ + void trySetBufferSize(size_t size); }; /** From fa7f0d6d07bdbedd06904d52bd111e58cb3d64c9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 16 Feb 2025 20:01:03 -0500 Subject: [PATCH 2/2] Allow setting `ssh://` pipe size Exposed for Hydra. We could make it fancier but with (a) new store settings (b) switch to `ssh-ng://` both in the works, it doesn't seem worth it. (cherry picked from commit 94a7c34b2f8285650e3130e5dc6ff5333eaa6dc8) --- src/libstore/legacy-ssh-store.cc | 3 +++ src/libstore/legacy-ssh-store.hh | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 3f62794ef..3849f088d 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -70,6 +70,9 @@ ref LegacySSHStore::openConnection() command.push_back(remoteStore.get()); } conn->sshConn = master.startCommand(std::move(command), std::list{extraSshArgs}); + if (connPipeSize) { + conn->sshConn->trySetBufferSize(*connPipeSize); + } conn->to = FdSink(conn->sshConn->in.get()); conn->from = FdSource(conn->sshConn->out.get()); diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh index 2444a7a66..92aa4ae56 100644 --- a/src/libstore/legacy-ssh-store.hh +++ b/src/libstore/legacy-ssh-store.hh @@ -30,6 +30,11 @@ struct LegacySSHStoreConfig : virtual CommonSSHStoreConfig */ Strings extraSshArgs = {}; + /** + * Exposed for hydra + */ + std::optional connPipeSize; + const std::string name() override { return "SSH Store"; } static std::set uriSchemes() { return {"ssh"}; }