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

Support "near" parameter in prepared query service block #2137

Merged
merged 15 commits into from
Jul 1, 2016
Merged

Conversation

ryanuber
Copy link
Member

Allows the use of the Near parameter in a prepared query definition. This parameter works just like the ?near=foo HTTP API parameter, only it is baked into the query, allowing it to be used from the DNS interface.

This enables some interesting functionality and opens up additional useful prepared query features when using DNS as the query mechanism.

/cc @slackpad

sortFrom = args.Origin
}

// Perform the distance sort
Copy link
Contributor

@sean- sean- Jun 21, 2016

Choose a reason for hiding this comment

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

Do we want to memoize this value for ~60s or whatever the interval is for the tomography calcs? I seem to recall someone saying this calc was "expensive," but if it's not, disregard. DNS can set a TTL, so maybe this isn't a huge issue, but that depends on the level of DNS caching at a given site.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not too bad - I'd probably leave as-is since this can be DNS TTL-ed and read-scaled with staleness across the servers.

// is executing the query on behalf of the client. It is taken
// as a QuerySource so that it can be used directly for queries
// relating to the agent servicing the request.
Origin QuerySource
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you take a look at the existing Source parameter right after this one? I think it was originally intended to do what Origin does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pretty close yeah. I originally tried to use that, but I switched to this because in the HTTP/DNS api code at the agent, you can't be sure what the prepared query actually does, so the agent can't be sure if it should pass its own name + datacenter, or the user's provided ?near and/or ?dc value. This just makes it so we can send both and decide outside of the agent.

@slackpad
Copy link
Contributor

@ryanuber took a look through and added some comments - this is looking good! I think the .Source thing is the biggest change since that's already there, otherwise some minor stuff.

// Build the query source. This can be provided by the client, or by
// the prepared query. Client-specified takes priority.
qs := args.Source
if qs.Datacenter == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

DC should always be supplied by parseDC but this is fine and defensive.

@slackpad
Copy link
Contributor

slackpad commented Jul 1, 2016

@ryanuber looks good! I still think we need to revert the parseSource() changes and the only other thing is to mention this in the docs. Otherwise, :shipit:.

@slackpad
Copy link
Contributor

slackpad commented Jul 1, 2016

👍

@ryanuber ryanuber merged commit ab16547 into master Jul 1, 2016
@ryanuber ryanuber deleted the f-pq-near branch July 1, 2016 19:28
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