Skip to content
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

Feat/use service with cache for reports #43

Merged
merged 18 commits into from
Jan 11, 2025

Conversation

Shelex
Copy link
Contributor

@Shelex Shelex commented Dec 5, 2024

Rationale

Issue:

  • implementation of some features for better visualization will lead to heavier impact on network interaction (especially s3)
  • working with huge s3 datasets causes a lot of querying under the hood
  • results could be huge files 200+ Mb

Solution:

  • as reports and results are just a static data, we can not only rely on client caching, but use in memory cache on backend
- UI -> Client cache -> Api route -> Storage -> S3/FS
+ UI -> Client cache -> Api route -> Service -> Cache/Storage -> S3/FS

To make it simple stupid, plain Map<string, entity> should work fine, so no database would be required in the app setup.

  • this allows us to have all parsed report data in the response and in future provide more data to the ui and quickly

  • this allows us to make interaction with s3 storage much efficient and avoid unnecessary queries

  • instead of reading entire file from formData into memory, utilize streams

Demo

S3

FS

BREAKING CHANGE

  • bump requirement for node version to v20

Changes

  • when generating reports we have a project name autocomplete which has reports just from selected results. however when you have results without project, to avoid typing manually or copy-paste it makes sense to provide report projects in the autocompletion list as well.
  • minor changes due to eslint errors/warnings

@Shelex Shelex marked this pull request as ready for review December 10, 2024 17:24
@Shelex Shelex force-pushed the feat/useServiceWithCacheForReports branch from d66d88e to afffd97 Compare December 16, 2024 20:51
@Shelex Shelex marked this pull request as draft December 17, 2024 08:26
fix: specify types for reports storage and cache
Token input is now password type (so autocomplete will work) (CyborgTests#44)

chore: increase readable stream watermark

chore: try direct stream pipe

chore: try streams manual event handling

chore: use more granular stream events

chore: check stream pipe with lower watermark

chore: lower watermark, use stream pipeline

chore: try converting writable to web

chore: reduce watermark, use pipeline

chore: try out limit watermark for writable stream as well

chore: ensure binary encoding for streams
chore: increase stream watermark to reduce cpu overhead

chore: check node v22

Revert "chore: check node v22"

This reverts commit 3dea917.
@Shelex Shelex force-pushed the feat/useServiceWithCacheForReports branch from 1bdc0cc to 6d0fbff Compare December 17, 2024 10:46
@Shelex Shelex marked this pull request as ready for review December 17, 2024 10:48
fix: handle s3 write for string content

chore: revert to custom readable stream for s3

chore: try downgrading docker node version

chore: explicitly turn off nextjs body parser for result upload

chore: remove obsolete next route segment config
@Shelex Shelex force-pushed the feat/useServiceWithCacheForReports branch from 8af9a93 to a7188cf Compare December 23, 2024 10:59
@Xotabu4 Xotabu4 self-requested a review January 11, 2025 19:41
@Xotabu4 Xotabu4 merged commit ffefea2 into CyborgTests:main Jan 11, 2025
4 checks passed
@Shelex Shelex deleted the feat/useServiceWithCacheForReports branch January 11, 2025 20:13
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.

3 participants