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

Public interfaces to support blocking queries for external dependencies #71

Closed
findkim opened this issue Oct 14, 2021 · 2 comments · Fixed by #73
Closed

Public interfaces to support blocking queries for external dependencies #71

findkim opened this issue Oct 14, 2021 · 2 comments · Fixed by #73
Labels
bug Something isn't working

Comments

@findkim
Copy link
Contributor

findkim commented Oct 14, 2021

Description

Custom dependency types external to the library cannot utilize blocking query functionality supported by hcat with BlockingQuery and QueryOptionsSetter interfaces as internal. And so the index query param is never set during the view fetch for CTS dependencies like ServicesRegex and CatalogServicesRegistration. This results in constant querying of the Consul API.

Expected Behavior

For example, the health service dependency is internal and satisfies both interfaces. The index is set on subsequent fetches as seen in the Consul logs.

    2021-10-14T17:24:33.549-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/health/service/api?filter=Checks.Status+%3D%3D+%22passing%22 from=127.0.0.1:52798 latency=5.96916ms
    2021-10-14T17:24:51.074-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/health/service/api?filter=Checks.Status+%3D%3D+%22passing%22&index=13 from=127.0.0.1:52798 latency=17.421803958s

Actual Behavior

Whereas the catalog services dependency is external (CTS) results in querying the endpoint continuously w/o blocking since the index query parameter is not set.

    2021-10-14T17:20:58.717-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=134.509µs
    2021-10-14T17:20:58.839-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=138.195µs
    2021-10-14T17:20:58.954-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=125.117µs
    2021-10-14T17:20:59.062-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=117.572µs
    2021-10-14T17:20:59.183-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=131.385µs
    2021-10-14T17:20:59.299-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=102.551µs
    2021-10-14T17:20:59.407-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=119.231µs
    2021-10-14T17:20:59.512-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=135.086µs
    2021-10-14T17:20:59.619-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=121.265µs
    2021-10-14T17:20:59.742-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=155.943µs
    2021-10-14T17:20:59.857-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=131.342µs
    2021-10-14T17:20:59.968-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=92.777µs
    2021-10-14T17:21:00.080-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=60.278µs
    2021-10-14T17:21:00.203-0500 [DEBUG] agent.http: Request finished: method=GET url=/v1/catalog/services from=127.0.0.1:52716 latency=91.272µs

Related to #67

@findkim findkim added the bug Something isn't working label Oct 14, 2021
@eikenb
Copy link
Contributor

eikenb commented Oct 15, 2021

The source of the issue with implementing the QueryOptionsSetter interface is that the argument it takes (QueryOptions) is private to the internal/dependency/ module. So the super quick fix would be to move QueryOptionsSetter and the QueryOptions struct into the public dep/ submodule. QueryOptionsSetter isn't strictly necessary, but it would make more sense.

I am leaning toward this currently as it would (I'm pretty sure) let us fix this issue in the short run while we think about how this could be better addressed before 1.0 (#67).

@findkim
Copy link
Contributor Author

findkim commented Oct 18, 2021

@eikenb that would work, thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants