Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

adds support for references in the descriptor #949

Merged
merged 9 commits into from
Oct 4, 2019
Merged
110 changes: 110 additions & 0 deletions waiter/docs/service-references.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# Service References

To process an HTTP request, Waiter needs to resolve a request to a service description.
This service description tells Waiter how to locate the service instance to use to process the current request.

Waiter uses request headers to construct the service description using the `ServiceDescriptionBuilder`.
It is possible for the _same_ service to be reachable from various combinations of headers:
1. token - single or multiple
1. on-the-fly services without tokens
1. on-the-fly services that extend tokens, e.g. run-as-requester

## Service GC

By default, Waiter relies on the service idle timeout to GC services after periods of inactivity (not receiving requests).
However, services reachable only via references, e.g. tokens, can be GC-ed eagerly if the reference has been updated.

**Note**: A service known to be referenced only via on-the-fly headers never goes stale.

When a service description is constructed from a request, the set of references for a service are also updated.
Individual references are available in the `:reference-type->entry` key in the descriptor,
the descriptor being built per request.
It is possible to reference the same service using different combinations of on-the-fly headers and tokens.
As such, there is a set of references (the union of all `:reference-type->entry` from the descriptor) that
can be used to reference a service.

### Reference (`:reference-type->entry` in the descriptor) examples

The `reference-type->entry` is a map where the keys represent a `:type` parameter used during staleness checks.

- A service created with on-the-fly header without tokens never goes stale.
It has an empty map as the value for `:reference-type->entry` in the descriptor:
```
{}
```

- A service created with `x-waiter-token: foo` header on a request will have the
following value for `:reference-type->entry` in the descriptor:
```
{:token {:sources [{:token "foo" :version "v0"}]}}
sradack marked this conversation as resolved.
Show resolved Hide resolved
```

- A service created with `x-waiter-token: bar,baz` header on a request will have the
following value for `:reference-type->entry` in the descriptor:
```
{:token {:sources [{:token "bar" :version "v1"}
{:token "baz" :version "v2"}]}}
```

As mentioned previously, as the same service can be accessed by another token, we end up
building a set of `reference-type->entry` maps as references that refer to the service.
E.g. if all the example requests above mapped to the same service, that service would have
the following references set:
```
#{{}
sradack marked this conversation as resolved.
Show resolved Hide resolved
{:token {:sources [{:token "foo" :version "v0"}]}}
{:token {:sources [{:token "bar" :version "v1"}
{:token "baz" :version "v2"}]}}}
```

## Staleness checks

The service GC processes all known references for a service and marks the service as a candidate for eager GC
if _all_ references used to access the service are stale.
sradack marked this conversation as resolved.
Show resolved Hide resolved
An individual reference, i.e. the `reference-type->entry` map computed in a request descriptor,
is stale if any of its value entries is stale.
This staleness check on the value is performed using the functions returned from
`retrieve-reference-type->stale-fn` of the builder and invoking the corresponding 'type' function on the value.

**Note**: A service known to be directly referenced (i.e. has an empty map among its references) never goes stale.

The default implementation of the `ServiceDescriptionBuilder` returns a map with a single entry for `:token`
from the `retrieve-reference-type->stale-fn`.
The provided `:token` staleness function deems services accessed via tokens to be stale if all tokens
used to access the service have been updated.

Custom builder implementations can add additional reference types to services and
need to provide appropriate staleness check functions for each reference type.
E.g. a hypothetical implementation which treats the image parameter as docker images can
have the following entry for a service:
```
{:image {:name "twosigma/kitchen" :build "20191001"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't we have multiple image labels resolving to the same service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they would show up as different entries in the references. The retrieve-reference-type->stale-fn is computed per request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the opportunity of my confusion to improve the docs and make this more explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation has been updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is much better now. Thank you for the updates.

:token {:sources [{"token" "foo" "version" "v1"}]}}
sradack marked this conversation as resolved.
Show resolved Hide resolved
```
The `retrieve-reference-type->stale-fn` must then provide an implementation for a function that
can check staleness of `:image` reference types.
E.g. if `retrieve-reference-type->stale-fn` returns:
```
{:image check-image-for-staleness-fn
:token check-token-for-staleness-fn}
```
and we have a service with the following set of references:
```
#{{:image {:name "twosigma/courier" :build "20191002"}}
{:image {:name "twosigma/kitchen" :build "20191001"}
:token {:sources [{:token "foo" :version "v0"}]}}
{:token {:sources [{:token "bar" :version "v1"}
{:token "baz" :version "v2"}]}}}
```
then the service goes stale when the following condition is true:
```
(and
# single image expression in the or since there was only one entry in the map
(or (check-image-for-staleness-fn {:name "twosigma/courier" :build "20191002"}))
# two expressions in the or, one for image and one for token
(or (check-image-for-staleness-fn {:name "twosigma/kitchen" :build "20191001"})
(check-token-for-staleness-fn {:sources [{:token "foo" :version "v0"}]}))
# single token expression in the or since there was only one entry in the map
(or (check-token-for-staleness-fn
{:sources [{:token "bar" :version "v1"} {:token "baz" :version "v2"}]})))
```
4 changes: 4 additions & 0 deletions waiter/integration/waiter/basic_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@
(is (= (:x-waiter-cmd headers) (get-in service-settings [:effective-parameters :cmd])))
(is (= "other" (get-in service-settings [:effective-parameters :metric-group])) service-id))

(let [service-settings (service-settings waiter-url service-id
:query-params {"include" "references"})]
(is (= [{}] (get service-settings :references)) (str service-settings)))

(testing "metric group should be other"
(is (= "other" (service-id->metric-group waiter-url service-id))
(str "Invalid metric group for " service-id)))
Expand Down
48 changes: 39 additions & 9 deletions waiter/integration/waiter/token_request_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,10 @@
(is (str/includes? service-id-1 name-string) (str "ERROR: App-name is missing " name-string))
(assert-service-on-all-routers waiter-url service-id-1 cookies)
(is (nil? (service-id->source-tokens-entries waiter-url service-id-1)))

(let [service-settings (service-settings waiter-url service-id-1 :query-params {"include" "references"})]
(is (= [{}] (get service-settings :references)) (str service-settings)))

(let [token (str "^SERVICE-ID#" service-id-1)
response (make-request-with-debug-info {:x-waiter-token token} #(make-request waiter-url "" :headers %))
service-id-2 (:service-id response)]
Expand All @@ -854,7 +858,12 @@
(is (= service-id-1 service-id-2) "The on-the-fly and token-based service ids do not match")
(assert-service-on-all-routers waiter-url service-id-1 cookies)
(is (= #{(make-source-tokens-entries waiter-url token)}
(service-id->source-tokens-entries waiter-url service-id-2)))))))))
(service-id->source-tokens-entries waiter-url service-id-2)))
(let [service-settings (service-settings waiter-url service-id-2 :query-params {"include" "references"})
references (set (get service-settings :references))]
(is (contains? references {}) (str service-settings))
(is (contains? references {:token {:sources [{:token token :version (token->etag waiter-url token)}]}})
(str service-settings)))))))))

(deftest ^:parallel ^:integration-fast test-namespace-token
(testing-using-waiter-url
Expand Down Expand Up @@ -1406,21 +1415,42 @@

(let [service-id-a (retrieve-service-id waiter-url {:x-waiter-token combined-token-header})
new-service-description (update first-service-description :cpus #(+ % 0.1))]

(let [service-settings (service-settings waiter-url service-id-a :query-params {"include" "references"})
references (set (get service-settings :references))]
(is (not (contains? references {})) (str service-settings))
(is (contains? references {:token {:sources [{:token token-name-a :version (token->etag waiter-url token-name-a)}
{:token token-name-b :version (token->etag waiter-url token-name-b)}]}})))

(let [response (post-token waiter-url (assoc new-service-description :token token-name-a))]
(assert-response-status response 200))

(let [service-id-b (retrieve-service-id waiter-url {:x-waiter-token combined-token-header})]

(let [service-settings (service-settings waiter-url service-id-b :query-params {"include" "references"})
references (set (get service-settings :references))]
(is (not (contains? references {})) (str service-settings))
(is (contains? references {:token {:sources [{:token token-name-a :version (token->etag waiter-url token-name-a)}
{:token token-name-b :version (token->etag waiter-url token-name-b)}]}})))

(let [response (post-token waiter-url (assoc new-service-description :token token-name-b))]
(assert-response-status response 200))

(let [service-id-c (retrieve-service-id waiter-url {:x-waiter-token combined-token-header})
service-a-details (service-settings waiter-url service-id-a)
service-b-details (service-settings waiter-url service-id-b)
service-c-details (service-settings waiter-url service-id-c)]
(is (nil? (:current-for-tokens service-a-details)))
(is (nil? (:current-for-tokens service-b-details)))
(is (= [token-name-a token-name-b]
(:current-for-tokens service-c-details))))))
(let [service-id-c (retrieve-service-id waiter-url {:x-waiter-token combined-token-header})]

(let [service-settings (service-settings waiter-url service-id-c :query-params {"include" "references"})
references (set (get service-settings :references))]
(is (not (contains? references {})) (str service-settings))
(is (contains? references {:token {:sources [{:token token-name-a :version (token->etag waiter-url token-name-a)}
{:token token-name-b :version (token->etag waiter-url token-name-b)}]}})))

(let [service-a-details (service-settings waiter-url service-id-a)
service-b-details (service-settings waiter-url service-id-b)
service-c-details (service-settings waiter-url service-id-c)]
(is (nil? (:current-for-tokens service-a-details)))
(is (nil? (:current-for-tokens service-b-details)))
(is (= [token-name-a token-name-b]
(:current-for-tokens service-c-details)))))))
(finally
(delete-token-and-assert waiter-url token-name-a)
(delete-token-and-assert waiter-url token-name-b))))))
Expand Down
Loading