-
Notifications
You must be signed in to change notification settings - Fork 7
Add proposals for signalling timeouts in TAMS #166
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
Open
j616
wants to merge
6
commits into
main
Choose a base branch
from
jamessa-timeouts
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
30e4aae
Add ADR0044 on signalling timeout periods
j616 0cf08c9
Add further options for negotiating timeouts. Select basic signaled t…
j616 0caed72
Implement ADR0044
j616 4fb8f8c
Fix broken cross-links in spec
j616 053ee77
Apply suggestions from code review
j616 42019bf
Apply agreed changes to minimum presinged URL and Object timeouts
j616 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| --- | ||
| status: "proposed" | ||
| --- | ||
| # Signalling of timeout periods | ||
|
|
||
| ## Context and Problem Statement | ||
|
|
||
| Following the creation of [ADR0043 Signalling Retention Time](../0043-signalling-retention-time.md), it was identified that there are two existing types of resource timeouts in TAMS that are not explicitly signalled. | ||
|
|
||
| The first is the garbage collection of Media Objects. | ||
| The spec is currently vague on how this should be implemented. | ||
| Objects are garbage collected both when they are no longer referenced by any Flow Segments, and if they are never registered against a Flow Segment. | ||
| Garbage collection where an Object is no longer referenced by a Flow Segment should happen immediately as permissions can no longer be derived from a parent Flow. | ||
| Garbage collection where an Object is never registered must happen after a period of time to allow for the opportunity for the Object to be registered against a Flow Segment. | ||
| Currently the specification only states the following: | ||
|
|
||
| > Service implementations need to handle situations where Objects were uploaded but no Flow Segment was registered successfully. | ||
|
|
||
| This means there are no explicit expectations on how much time a client has to make use of an Object. | ||
|
|
||
| Secondly, TAMS currently makes no recommendations on the expiry time of pre-signed URLS. | ||
| Common approaches include this information in the URL itself. | ||
| But this cannot be relied upon. | ||
| This means there are no explicit expectations on how much time a client has to make use of a pre-signed URL. | ||
|
|
||
| This ADR revisits both of these topics and considers potential approaches to provide explicit expectations regarding these timeouts. | ||
|
|
||
| ## Considered Options | ||
|
|
||
| * Option 1a: Signal Object garbage collection timeout via the `/service` metadata | ||
| * Option 1b: Specify a fixed Object garbage collection timeout in the specification | ||
| * Option 1c: Do not specify an Object garbage collection timeout | ||
| * Option 1d: Client and Service negotiate Object garbage collection timeout | ||
| * Option 2a: Signal presigned URL expiry time via the `/service` metadata | ||
| * Option 2b: Specify presigned URL expiry time in the specification | ||
| * Option 2c: Do not specify a presigned URL expiry time | ||
| * Option 2d: Client and Service negotiate presigned URL expiry time | ||
|
|
||
| ## Decision Outcome | ||
|
|
||
| Chosen option: Option 1a, and Option 2a. | ||
|
|
||
| These options will be implemented such that the API specification could be extended in future to support Options 1d, and 2d if required. | ||
|
|
||
| The minimum timeout for presigned URLs has been selected as 30 seconds. | ||
| This is to allow for long distance, poor quality connections. | ||
| In particular, latency to opposite sides of Earth is on the order of 250ms and can be much higher. | ||
|
|
||
| The minimum timeout for Media Objects has been selected as 5 minutes. | ||
| This is to allow for the upload of moderately sized Objects over poor quality connections. | ||
|
|
||
| The timeout for Objects is significantly higher than presigned URLs as uploads/downloads must be initiated with presigned URLs before they time out, but Object upload and registration against Flow Segments must be completed before the Object timeout. | ||
|
|
||
| ### Implementation | ||
|
|
||
| Implemented in [PR #166](https://github.com/bbc/tams/pull/166) | ||
|
|
||
| ## Pros and Cons of the Options | ||
|
|
||
| ### Option 1a: Signal Object garbage collection timeout via the `/service` metadata | ||
|
|
||
| This option would see a parameter added to the metadata at the `/service` endpoint that a Service shall use to communicate the minimum time Objects will be available for after first creation. | ||
| Clients should upload content to Objects and register them against segments within this timeframe. | ||
| The specification would include a minimum value allowing Clients to validate they meet a minimum level of performance. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe in which Objects must be used | ||
| * Good, because it allows for Services to set that timeframe based on their implementation/requirements | ||
| * Good, because it provides clear performance requirements for Clients | ||
| * Bad, because it requires Clients to adapt to that signalled timeframe (or at least meet the minimum level) | ||
|
|
||
| ### Option 1b: Specify a fixed Object garbage collection timeout in the specification | ||
|
|
||
| This option would see the TAMS specification specify the minimum time Objects will be available for after first creation. | ||
| Clients should upload content to Objects and register them against segments within this timeframe. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe in which Objects must be used | ||
| * Good, because it doesn't require Clients to adapt to a signalled timeframe | ||
| * Bad, because it doesn't allow for Services to set that timeframe based on their implementation/requirements | ||
| * Bad, because this "minimum" would not allow Clients to rely on availability beyond the specified time, even if the Service allows use significantly beyond the specified timeframe | ||
|
|
||
| ### Option 1c: Do not specify an Object garbage collection timeout | ||
|
|
||
| This option would see no changes to the TAMS specification. | ||
| The current statement in the specification - that service implementations should handle the case where Objects aren't registered - would remain. | ||
|
|
||
| * Good, because it allows for Services to set that timeframe based on their implementation/requirements | ||
| * Bad, because there is no clear contract between Services and Clients regarding the timeframe in which Objects must be used | ||
| * Bad, because Clients cannot adapt to this potentially variable timeframe | ||
|
|
||
| ### Option 1d: Client and Service negotiate Object garbage collection timeout | ||
|
|
||
| This option would see Clients and Services actively negotiate Object garbage collection timeouts. | ||
| This would likely take the form of minimum and maximum values being advertised by the Service at its `/service` endpoint. | ||
| Clients would then specify a value in this range when requesting Object allocation. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe in which Objects must be used | ||
| * Good, because it allows for Services to set the permitted range of timeframes based on their implementation/requirements | ||
| * Good, because it allows Clients to to request a timeframe appropriate to its requirements | ||
| * Bad, because it requires Clients and Services to adapt to that negotiated timeframe | ||
| * Bad, because it adds significant complexity to the API and implementations | ||
|
|
||
| ### Option 2a: Signal presigned URL expiry time via the `/service` metadata | ||
|
|
||
| This option would see a parameter added to the metadata at the `/service` endpoint that a Service shall use to communicate the minimum time pre-signed URLs will be valid for. | ||
| The specification would include a minimum value allowing Clients to validate they meet a minimum level of performance. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe over which pre-signed URLs are valid. | ||
| * Good, because it allows for Services to set that timeframe based on their implementation/requirements | ||
| * Good, because it provides clear performance requirements for Clients | ||
| * Bad, because it requires Clients to adapt to that signalled timeframe | ||
|
|
||
| ### Option 2b: Specify presigned URL expiry time in the specification | ||
|
|
||
| This option would see the TAMS specification specify the minimum time pre-signed URLs will be valid for. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe over which pre-signed URLs are valid. | ||
| * Good, because it doesn't require Clients to adapt to a signalled timeframe | ||
| * Bad, because it doesn't allow for Services to set that timeframe based on their implementation/requirements | ||
| * Bad, because this "minimum" would not allow Clients to rely on availability beyond the specified time, even if the Service allows use significantly beyond the specified timeframe | ||
|
|
||
| ### Option 2c: Do not specify a presigned URL expiry time | ||
|
|
||
| This option would see no changes to the TAMS specification. | ||
|
|
||
| * Good, because it allows for Services to set that timeframe based on their implementation/requirements | ||
| * Bad, because there is no clear contract between Services and Clients regarding the timeframe over which pre-signed URLs are valid | ||
| * Bad, because Clients cannot adapt to this potentially variable timeframe | ||
|
|
||
| ### Option 2d: Client and Service negotiate presigned URL expiry time | ||
|
|
||
| This option would see Clients and Services actively negotiate the time pre-signed URLs will be valid for. | ||
| This would likely take the form of minimum and maximum values being advertised by the Service at it's `/service` endpoint. | ||
| Clients would then specify a value in this range when requesting pre-signed URLs. | ||
|
|
||
| * Good, because it provides a clear contract between Services and Clients regarding the timeframe over which pre-signed URLs are valid. | ||
| * Good, because it allows for Services to set the permitted range of timeframes based on their implementation/requirements | ||
| * Good, because it allows Clients to to request a timeframe appropriate to its requirements | ||
| * Bad, because it requires Clients and Services to adapt to that negotiated timeframe | ||
| * Bad, because it adds significant complexity to the API and implementations |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.