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.

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

gvfs-helper: add gvfs.fallback config option #664

Closed

Conversation

derrickstolee
Copy link
Collaborator

By default, GVFS Protocol-enabled Scalar clones will fall back to the origin server if there is a network issue with the cache servers. However (and especially for the prefetch endpoint) this may be a very expensive operation for the origin server, leading to the user being throttled. This shows up later in cases such as 'git push' or other web operations.

To avoid this, create a new config option, 'gvfs.fallback', which defaults to true. When set to 'false', pass '--no-fallback' from the gvfs-helper client to the child gvfs-helper server process.

This will allow users who have hit this problem to avoid it in the future. In case this becomes a more widespread problem, engineering systems can enable the config option more broadly.

Enabling the config will of course lead to immediate failures for users, but at least that will help diagnose the problem when it occurs instead of later when the throttling shows up and the server load has already passed, damage done.


  • This change only applies to interactions with Azure DevOps and the
    GVFS Protocol.

By default, GVFS Protocol-enabled Scalar clones will fall back to the
origin server if there is a network issue with the cache servers.
However (and especially for the prefetch endpoint) this may be a very
expensive operation for the origin server, leading to the user being
throttled. This shows up later in cases such as 'git push' or other web
operations.

To avoid this, create a new config option, 'gvfs.fallback', which
defaults to true. When set to 'false', pass '--no-fallback' from the
gvfs-helper client to the child gvfs-helper server process.

This will allow users who have hit this problem to avoid it in the
future. In case this becomes a more widespread problem, engineering
systems can enable the config option more broadly.

Enabling the config will of course lead to immediate failures for users,
but at least that will help diagnose the problem when it occurs instead
of later when the throttling shows up and the server load has already
passed, damage done.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
@dscho
Copy link
Member

dscho commented Jun 28, 2024

I wonder whether this would merit a new test case in t5799-gvfs-helper.sh to ensure that the config option keeps working as intended. @jeffhostetler what are your thoughts on that?

@jeffhostetler
Copy link

Let me look at the test suite now.

strvec_push(&argv, "--cache-server=trust");
strvec_pushf(&argv, "--shared-cache=%s",
gh_client__chosen_odb->path);

/* If gvfs.fallback=false, then don't add --fallback. */

Choose a reason for hiding this comment

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

Would it be easier to just hack gvfs-helper.exe to lookup that config value ??

Copy link
Member

Choose a reason for hiding this comment

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

Might be too magic, though. And so far, gvfs-helper only uses git_config(git_default_config), no custom config variables there.

Choose a reason for hiding this comment

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

d'oh, nevermind.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moreover, the client passes --fallback to the server directly, which would normally override any config option.

@jeffhostetler
Copy link

Please see #665

@dscho
Copy link
Member

dscho commented Jul 1, 2024

@derrickstolee if you're okay with #665, I'd like to take that and close this here PR?

@derrickstolee
Copy link
Collaborator Author

Closing in favor of #665.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants