Fix client disconnect when queryPathInfo() returns a negative result#13784
Fix client disconnect when queryPathInfo() returns a negative result#13784
Conversation
…alid paths This caused RemoteStore::queryPathInfoUncached() to mark the connection as invalid (see RemoteStore::ConnectionHandle::~ConnectionHandle()), causing it to disconnect and reconnect after every lookup of an invalid path. This caused huge slowdowns in conjunction with 19f89eb and lazy-trees.
github-actions
bot
added
the
store
| try { | ||
| std::shared_ptr<const ValidPathInfo> info; | ||
| { | ||
| auto info = ({ |
There was a problem hiding this comment.
Can this use an immediatelly invoked lambda instead of the GNU extension?
[&](){
auto conn(getConnection());
return conn->queryPathInfo(*this, &conn.daemonException, path);
}();This is standard C++ at least and not too much worse than the GNU extension.
There was a problem hiding this comment.
Well, we're already using statement exprs quite a bit in Nix, and they work fine in both gcc and clang.
There was a problem hiding this comment.
Fair enough, yeah. But I was under the impression that is polyfill from the time before C++ had conveniences for this stuff.
There was a problem hiding this comment.
I made lots of the ({...}), and I agree that calling the lambda is better, because I think some rvalue/lvalue stuff is better defined.
(Also, because #9527 someday :))
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
A, nevermind I was looking at the wrong thing. |
|
Thanks for fixing my bug! |
4 participants
Motivation
This fixes a bug in
RemoteStore::queryPathInfoUncached()where the client disconnects from the daemon if the path is invalid. This is becauseWorkerProto::BasicClientConnection::queryPathInfo()throws an exception, which causesRemoteStore::ConnectionHandle::~ConnectionHandle()to unnecessarily drop the connection. These disconnects/reconnects can cause big slowdowns for the client.Since this has the side-effect of fixing negative path info lookup caching for remote stores, we now need to make sure to invalidate path info cache entries in
RemoteStore::addToStoreFromDump().Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.