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

Misleading wording on profile resolver.defaults.retry #1070

Open
hannaeko opened this issue May 18, 2022 · 2 comments · May be fixed by #1258
Open

Misleading wording on profile resolver.defaults.retry #1070

hannaeko opened this issue May 18, 2022 · 2 comments · May be fixed by #1258
Assignees
@tgreenx
Labels
S-PRforIssue Status: There is a PR that is meant to resolve the issue T-Question Type: Incoming question
Milestone
v2024.2

Comments

@hannaeko
Copy link
Member

hannaeko commented May 18, 2022 •
edited
Loading

As reported on the mailing list, in the profile, retry refers to the total number of tries before the request is considered failed and should probably not include the initial query.

For instance the dig manual states

+tries=T
This option sets the number of times to try UDP and TCP queries to
server to T instead of the default, 3. If T is less than or equal
to zero, the number of tries is silently rounded up to 1.

+retry=T
This option sets the number of times to retry UDP and TCP queries to
server to T instead of the default, 2. Unlike +tries, this does not
include the initial query.

Should we make that distinction in the profile as well? Or at least make it so that retry does not count the initial attempt?

As we pass this parameter to LDNS as is, is this an issue that could better be resolved in Zonemaster::LDNS?
@hannaeko hannaeko added the T-Question Type: Incoming question label May 18, 2022
@matsduf matsduf added this to the v2022.2 milestone May 21, 2022
@matsduf matsduf modified the milestones: v2022.2, v2023.1 Dec 20, 2022
@matsduf matsduf modified the milestones: v2023.1, v2023.2 Jun 26, 2023
@tgreenx
Copy link
Contributor

tgreenx commented Jul 19, 2023

I would vote for keeping the current variable name (retry), make it not count the initial query, and change its documentation; it has the benefits of being in line with the LDNS functions, and a minimal change to Zonemaster-LDNS/Engine.

@tgreenx tgreenx self-assigned this Jul 19, 2023
tgreenx added a commit to tgreenx/zonemaster-engine that referenced this issue Jul 19, 2023
@tgreenx
Update resolver.defaults.retry - zonemaster#1070
3f5da4c 
This commit updates "resolver.defaults.retry" profile parameter by making it more proper to its name:

- Make it exclude the initial query.
  Note that the actual value of "resolver.defaults.retry" is silently incremented before being passed to Zonemaster-LDNS. This is needed because LDNS does count the initial query as a try with this parameter.
- Update default values
- Update documentation
- Update unitary tests
tgreenx added a commit to tgreenx/zonemaster-engine that referenced this issue Jul 19, 2023
@tgreenx
Update resolver.defaults.retry - zonemaster#1070
13825aa 
This commit updates "resolver.defaults.retry" profile parameter by making it more proper to its name:

- Make it exclude the initial query.
  Note that the actual value of "resolver.defaults.retry" is silently incremented before being passed to Zonemaster-LDNS. This is needed because LDNS does count the initial query as a try with this parameter.
- Update default values
- Update documentation
- Update unitary tests
@tgreenx tgreenx linked a pull request Jul 19, 2023 that will close this issue
Open
@tgreenx tgreenx linked a pull request Jul 19, 2023 that will close this issue
Update profile parameter "resolver.defaults.retry" #1258
Open
@tgreenx
Copy link
Contributor

tgreenx commented Jul 19, 2023

I have proposed #1258 as a solution.

@tgreenx tgreenx added the S-PRforIssue Status: There is a PR that is meant to resolve the issue label Jul 19, 2023
@matsduf matsduf modified the milestones: v2023.2, v2024.1 Mar 19, 2024
@matsduf matsduf modified the milestones: v2024.1, v2024.2 Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tgreenx tgreenx

Labels
S-PRforIssue Status: There is a PR that is meant to resolve the issue T-Question Type: Incoming question
Projects
None yet
Milestone
v2024.2
Development

Successfully merging a pull request may close this issue.

Update profile parameter "resolver.defaults.retry" tgreenx/zonemaster-engine
3 participants
@matsduf @hannaeko @tgreenx