-
Notifications
You must be signed in to change notification settings - Fork 11
Dumps API - Make dump creation an asynchronous task #139
Conversation
🚨 Breaking API change detected: Modified (28)
Removed (1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks clear and good to me 👍
If he has the time I would love to see a review from @nicolasvienot though because that's quite a big breaking for the cloud 😬
My only concern is that a user asking for a dump will potentially wait hours before having the dump if a lot of tasks are in front in the dump. I don't know if this is problematic. Apart for that, great spec! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec is clear, thanks @gmourier!
A dump creation is now an asynchronous task. This means that the dump creation task does not take priority over other tasks and that the dump will be created when the task store will not have any other processing task or when other enqueud tasks are not positioned before.
As @irevoire and @bidoubiwa noted, the fact that we can't prioritize a dump anymore and need to wait until all the previous tasks are processed is quite an issue as it means that we won't be able to trigger an update directly on the cloud solution.
What's your opinion on this @qdequele ?
Hello @gmourier, after a discussion with @nicolasvienot about implementing the update feature (the ability to update a meilisearch version on the cloud manually) by the cloud team, we have some comments on this specification. tl;dr: Explanation:
The goal is to update the server as fast as possible because the user will not be able to index new documents during the entire process. In a way, we don't want to wait until all tasks are finished because this time could be very long. |
e2aa0f0
to
fe690fb
Compare
7bdf732
to
283b128
Compare
Hey @meilisearch/cloud-team 👋 I updated the specification to mention that, like today, the dumps task will be prioritized before the other enqueued tasks. (the visual order of tasks won't be changed in the It will run as soon as the current processing task finishes, even if others have been enqueued before. @Kerollmops it could also bypass the current processing task and set a global read transaction to fetch the current state of the B+ Tree of the different LMDBs (indexes) of a Meilisearch instance while a write transaction is in progress. What do you think about it? (This is for future consideration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌
Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>
548f569
to
c81dac0
Compare
* wip * Make a dump creation a visible asynchronous task * Add precisions * Update open-api.yml * Add ommited type field for summarized task response * Add future possibilities * Apply suggestions from code review Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Precise that indexUid can be null * Precise priorization of dumpCreation task over other task types * Keep taskUid for 202 response * remove dumps.get API key action Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>
* Bump open-api.yml to v0.28 * Telemetry - Add `x-meilisearch-client` query parameter (#145) * Introduce the x-meilisearch-client query parameter * Update text/0034-telemetry-policies.md Co-authored-by: Bruno Casali <brunoocasali@gmail.com> * Update text/0034-telemetry-policies.md Co-authored-by: Bruno Casali <brunoocasali@gmail.com> Co-authored-by: Bruno Casali <brunoocasali@gmail.com> * GeoSearch — Support string type for `_geo` `lat` and `lng` fields (#83) * Update specification to support string type for _geo lat and lng fields * mention types mixing for _geo object * Tasks API - Rename `uid` to `taskUid` in the `202 - Accepted` Summarized Task Response (#144) * Rename 202 uid to taskUid * Update text/0060-tasks-api.md Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Tasks API - Seek/Keyset based pagination (#115) * Move cursor based pagination spec to tasks API spec * remove pagination as a future capability * Clarify boundaries for limit query parameter * Update OpenApi specification * Remove limit field boundaries * Apply suggestions from code review Co-authored-by: Clément Renault <renault.cle@gmail.com> * Update open-api.yml and removes cursor term mentions * Update text/0060-tasks-api.md Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com> * Remove route mention in seek-keyset pagination section Co-authored-by: Clément Renault <renault.cle@gmail.com> Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com> * Tasks API - Filter tasks list by `type`/`status`/`indexUid` (#116) * move filtering tasks by status/type parameter to task api spec * Update specification * Add details about case-sensitivy + rework error message * Introducing naming changes plus make the specification a source of truth instead of a changelog * Remove a future possibility being introduced * misc - replace createIndex to the right type and add the missing type field to the 202 Response resource * Dumps API - Make dump creation an asynchronous task (#139) * wip * Make a dump creation a visible asynchronous task * Add precisions * Update open-api.yml * Add ommited type field for summarized task response * Add future possibilities * Apply suggestions from code review Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Precise that indexUid can be null * Precise priorization of dumpCreation task over other task types * Keep taskUid for 202 response * remove dumps.get API key action Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Search API - Remove/Rename confusing fields (#135) * Rename nbHits, remove exhaustive* boolean fields * Rename approximativeNbHits to estimatedTotalHits * Update open-api.yaml * Apply naming changes for facet distribution and showing matches position * Add a telemetry for facet distribution usage * API Guideline - Return list of API resources under a `results` array (#138) * Place list of documents under a results array on /documents * Add results array for indexes object list * Add the future of indexes pagination * Update open-api.yml * Fix typo * Apply suggestions from code review Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Add offset/limit pagination for indexes and API keys * Try to add multipe refs to a response object Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> * Remove name field (#140) * Documents API - `displayedAttributes` should not impact the documents API / Rename `attributesToRetrieve` to `fields` (#143) * Specifies that displayedAttributes setting does not impact the GET documents endpoint * Rename attributeToRetrieve to fields on /documents * Add a future possibily to rejectt a field from a document in the given response * Precise behavior details about fields query parameter * Add fields query parameter on GET /indexes/{index}/documents/{docId} * API Keys - Determinist API Keys + Security changes (#148) * Add an uid to make API Keys determinists, plus add a non-unique human readable name field to ease reading information * Describe errors for uid and name fields * Apply suggestions from code review Co-authored-by: Bruno Casali <brunoocasali@gmail.com> * misc: add precisions * Reorganize route descriptions * Update error_code when API Key already exists for a given uid * Apply suggestions from code review Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> * Add new keys actions, remove master-key changes, introduce a new error for immutable field and update tenant token * Update open-api spec * Update immutable_field error message * Apply suggestions from code review Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> * Mention that the Default Admin API Key can manage keys * Specify that the JWT Tenant Token must be enrypted with the API Key value * Update the spec regarding the description of the Admin API Key to be up-to-date * Add uid_or_key url param to update and delete a key * Update text/0085-api-keys.md Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> * Update text/0085-api-keys.md Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> Co-authored-by: Bruno Casali <brunoocasali@gmail.com> Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> Co-authored-by: Kerollmops <clement@meilisearch.com> * Geosearch - Enhance lat/lng format error messages (#149) * Update the geosearch error message * misc: organiser error message in the right specification Co-authored-by: Guillaume Mourier <guillaume@meilisearch.com> * Introduces HTTP Verbs changesto be compliant regarding a Rest API (#152) * Telemetry - Replace `x-meilisearch-client` query parameter by `X-Meilisearch-Client` header (#150) * Removes x-meilisearch-client, replace it by a header * Remove capslock * fix typo (#151) * Mention telemetry.meilisearch.com (#153) * update ranking rules error message (#154) * Misc — Update dump versions compatibility table (#156) * Update dump table * Update text/0105-dumps-api.md * Settings API - Customize the hard limits for `pagination` and `faceting` (#157) * Introduces specification files * Update files name * branch telemetry * Update open-api.yml * Update text/0034-telemetry-policies.md Co-authored-by: Clément Renault <renault.cle@gmail.com> * update open-api.yml * Update text/157-faceting-setting-api.md Co-authored-by: Clément Renault <renault.cle@gmail.com> * Rename limitedTo to maxTotalHits * Specify order of returned facet Co-authored-by: Clément Renault <renault.cle@gmail.com> * Add dumpCreation task type to OpenAPI.yml * Tasks filtering params to be in query instead of path on OpenAPI spec Co-authored-by: Bruno Casali <brunoocasali@gmail.com> Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com> Co-authored-by: Clément Renault <renault.cle@gmail.com> Co-authored-by: Tommy <68053732+dichotommy@users.noreply.github.com> Co-authored-by: Many the fish <legendre.maxime.isn@gmail.com> Co-authored-by: Kerollmops <clement@meilisearch.com> Co-authored-by: Tamo <tamo@meilisearch.com> Co-authored-by: ad hoc <postma.marin@protonmail.com> Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
🤖 API Diff
Motivation
Today, the route to retrieve the status of a dump is not consistent with other APIs that retrieve the details of a resource and it does not share the same values for the
status
enum comparing to atask
status
.Rather than reworking the dump API endpoints, we propose to make dumps an asynchronous task like the others.
This allows to have a history of dumps created for free and to unify the status directly as well as to provide a way to see the status of a dump creation from its associated
task
.Changes
type
associated isdumpCreation
.details
object of a task under thedumpUid
field.dump_already_processing
error when a dump is already processed since they can now be enqueued.POST
on/dumps
return a202 Accepted
representing a summarizedtask
.202 Accepted
(Summarized task response) and the relatedtask
object define theindexUid
tonull
since dump are related to all indexes.dumpCreation
tasks are only visible on GET/tasks
and/task/task_uid
since they are not related to a specific index./dumps/:dump_uid/status
is removed.dumps.get
API Key action.Misc
type
for202 Accepted
response in the open-api.yml🚨
uid
totaskUid
in the202 - Accepted
Summarized Task Response #144 (comment)