Skip to content

Conversation

@ketan96-m
Copy link
Collaborator

@ketan96-m ketan96-m commented Oct 5, 2024

Resolves #468

Problem

When the client submits the request but encounters client failure, there is no way of resuming the transforms from the last point. The client is forced to rerun the request which triggers the backend again.

Fix

This PR fixes the broken client problem. The client will be able to resume the request from the last point. Given the client submits the same request again (important since the request hash is calculated and compared with any pending requests)
Future scope:

  • Provide a CLI option to rerun all the jobs which are stale due to client failures
  • Download the files which are already processed by the backend by providing a request id (CLI)

Approach

Created a new instance of tinydb whose sole purpose is to keep track of the transform request submissions.
I had a discussion with @ponyisi whether we should have the cache use additional columns and flags. The cache currently holds the TransformationResults AFTER the request is processed. We could technically have additional columns, but it would mean changing previous cache specific functions.
I propose to create a new instance of tinydb which stores the submitted transform request ( I have named it as queue). This will allow to add transform request submission related functions to be separate from the cache features. Thus keeping the existing code intact and only adding the necessary steps in between.
Also this will allow us to add the future scope (CLI) option without much modification.

Open to design inputs

Details of this PR

  • Created a separat instance of tinydb called queue which records each new transform_request
  • Incase of multiple transform request with same hash only one record is maintained and only one submission to backend is created saving redundant requests
  • Incase of failure on the client side, the queue resumes when you run the same request again. However, if you submit is with 'ignore cache` parameter, it will rerun the request to the backend.

@ketan96-m ketan96-m marked this pull request as draft October 5, 2024 05:23
@ketan96-m ketan96-m self-assigned this Oct 5, 2024
@ketan96-m ketan96-m marked this pull request as ready for review October 5, 2024 18:37
@codecov
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.11%. Comparing base (7a65b87) to head (40ad11d).
Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   83.10%   84.11%   +1.01%     
==========================================
  Files          26       26              
  Lines        1415     1505      +90     
==========================================
+ Hits         1176     1266      +90     
  Misses        239      239              
Flag Coverage Δ
unittests 84.11% <100.00%> (+1.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ketan96-m ketan96-m changed the title Resume request when client fails Resume request when client fails fixes #468 Oct 5, 2024
@ketan96-m
Copy link
Collaborator Author

ketan96-m commented Oct 6, 2024

@ponyisi @BenGalewsky @gordonwatts ready for review

@ketan96-m ketan96-m requested review from BenGalewsky, gordonwatts and ponyisi and removed request for BenGalewsky and gordonwatts October 7, 2024 13:20
@ketan96-m ketan96-m requested a review from gordonwatts October 7, 2024 13:20
@BenGalewsky
Copy link
Contributor

I'm sorry, @ketan96-m - I don't agree with @ponyisi on this... adding a new database just to avoid updating existing code is just a recipe for technical debt. Please add a new boolean column for completed which indicates if this request has reached an end one way or another. That way you can add entries as soon as they are submitted.

@ponyisi
Copy link
Collaborator

ponyisi commented Oct 23, 2024

Superseded by #497

@ponyisi ponyisi closed this Oct 23, 2024
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.

Frontend crashes while working transform is in progress are not saved

4 participants