Fix CSV export causing prod restarts by streaming response#707
Fix CSV export causing prod restarts by streaming response#707Saurabhsing21 wants to merge 2 commits intojlab-sensing:mainfrom
Conversation
- Replace pandas/in-memory CSV generation with streaming generator in /api/cell/datas - Push resampling/aggregation into SQL (date_trunc + avg) and iterate DB results in chunks - Update frontend to download CSV as a blob and stop polling /api/status for export results - Add/expand tests to cover multi-cell export, void-filling, and hour resampling Signed-off-by: Saurabhsing21 <saurabhsingh881888@gmail.com>
|
Hey @aleclevy can you review this when you are free |
|
Hey @Saurabhsing21, just wanted to inform you so you don't waste time debugging the CI logs: those failures aren't from your code! There's a global lint error in main right now, and the backend tests are deadlocking on everyone's PRs. I've got a hotfix open for the linter and flagged the hanging tests to the maintainers. |
|
At a quick glance the approach seems reasonable. I put both me and Alec on as reviewers. Its currently finals week for us so we are both bandwidth limited atm. Expect a review early next week by Tuesday from one of us. |
|
Hey @Saurabhsing21, as the maintainers are busy with finals next week, so to get the CI pipeline unblocked for your PR and everyone else's, I extracted your conftest.py fix into a standalone hotfix PR! I made sure to credit you for the fix. Once they merge it, your backend tests should finally turn green! |
Sure, Thanks |
Ok , thanks |
hotfix: resolve pytest db deadlock (extracted from Saurabhsing21 PR #707
|
/deploy |
|
PR deployed to development server Commit: 707 Triggered by: @jmadden173 |
jmadden173
left a comment
There was a problem hiding this comment.
The implementation is sound and checked on our dev server. Couple comments and TODOs before merging. Let me know if anything needed clarification.
TODO:
- Remove these lines before we merge to say that we've fixed the issue.
ENTS-backend/frontend/src/pages/dashboard/Dashboard.jsx
Lines 577 to 596 in 4e0acbb
- Update the changelog with a link to the issue
- Merge from upstream to fix conflicts and linting issue.
| sensor = Sensor.query.filter_by( | ||
| name="phytos31", | ||
| measurement="voltage", | ||
| cell_id=cell_id, | ||
| ).first() |
There was a problem hiding this comment.
Instead of the single "phytos31" sensor, it should query the available sensors and data from the table and add the necessary columns.
| CSV_HEADERS = [ | ||
| "cell_id", | ||
| "cell_name", | ||
| "timestamp", | ||
| "vwc", | ||
| "temp", | ||
| "ec", | ||
| "raw_vwc", | ||
| "v", | ||
| "i", | ||
| "p", | ||
| "data", | ||
| "measurement", | ||
| "unit", | ||
| "type", | ||
| ] |
There was a problem hiding this comment.
The CSV headers are not fixed. Based on how many "sensors" there are it will dynamically change the number of columns.
|
|
||
| # Ensure large exports don't get fully buffered in memory by the DB driver. | ||
| result = db.session.execute(stmt.execution_options(stream_results=True)).yield_per( | ||
| 1000 |
There was a problem hiding this comment.
Lets make the 1000 a top level variable in the module that can be tuned if we run into issues in the future.
|
@Saurabhsing21 Any way you could take a look at these? |
Fix : #668
Summary
Fixes the production “Export to CSV” crash/restart by replacing the old in-memory CSV generation flow with a true streaming CSV response from the backend. Frontend is updated to download the streamed file directly as a blob (no Celery status polling).
Problem
Clicking Export to CSV in production caused the site to become unresponsive and the backend to restart (Gunicorn worker timeout / SIGKILL, likely OOM). The previous implementation built large datasets/CSVs in memory.
Solution
GET /api/cell/datasnow streams CSV rows incrementally (constant memory footprint).date_trunc+avgfor resampling) instead of pandas/in-memory merges.yield_per+ streaming execution options) to avoid buffering large result sets.responseType: bloband uses theContent-Dispositionfilename.Key Changes
Backend
backend/api/resources/cell_data.pyto:Response(stream_with_context(...)))"void"vwc_unit="%",raw_vwc_unit="raw"inbackend/api/models/teros_data.pybackend/tests/conftest.pysupportsTEST_SQLALCHEMY_DATABASE_URIFrontend
frontend/src/services/cell.js: request/api/cell/datasasblob, parse filename fromContent-Dispositionfrontend/src/pages/dashboard/components/DownloadBtn.jsx: simplified export flow to a single streamed downloadTests
Automated
python3 -m pytest -q backend/tests57 passed, 7 skippedbackend/tests/test_ttn.pyis skipped unless realTTN_API_KEY/TTN_APP_IDare provided (integration tests hit the real TTN API).npm run lint(pass)npm test -- --run(pass)npm run build(pass)Manual / Runtime
Transfer-Encoding: chunked)WORKER TIMEOUT/SIGKILL/ restart