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

Be able to index a resource by storage referece #2094

Merged
merged 18 commits into from
May 2, 2024

Conversation

lferran
Copy link
Contributor

@lferran lferran commented Apr 24, 2024

Description

This way we can have sidecar indexing on more KBs at the same time than now because memory is a problem as we have to load the resources in memory.

How was this PR tested?

Describe how you tested this PR.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 728443a Previous: 1a9ce6c Ratio
nucliadb/search/tests/unit/search/test_fetch.py::test_highligh_error 7770.0872950681505 iter/sec (stddev: 1.9786275859340796e-7) 13339.058637295573 iter/sec (stddev: 1.296413027597777e-7) 1.72

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: d6ad4f9 Previous: d4afd82 Ratio
nucliadb/search/tests/unit/search/test_fetch.py::test_highligh_error 7619.282909228837 iter/sec (stddev: 1.660689890135406e-7) 13028.533525895236 iter/sec (stddev: 4.192637045977425e-7) 1.71

This comment was automatically generated by workflow using github-action-benchmark.

Copy link

codecov bot commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.78%. Comparing base (ea225f3) to head (0612c92).
Report is 20 commits behind head on main.

❗ Current head 0612c92 differs from pull request most recent head 728443a. Consider uploading reports for the commit 728443a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2094      +/-   ##
==========================================
- Coverage   82.09%   74.78%   -7.32%     
==========================================
  Files          42       80      +38     
  Lines        2346     5945    +3599     
==========================================
+ Hits         1926     4446    +2520     
- Misses        420     1499    +1079     
Flag Coverage Δ
ingest 70.05% <ø> (?)

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.

@lferran lferran changed the title Download resource from index node Be able to index a resource by storage referece Apr 26, 2024
@lferran
Copy link
Contributor Author

lferran commented Apr 26, 2024

[sc-9954]

@lferran lferran marked this pull request as ready for review April 26, 2024 14:20
@lferran lferran requested a review from a team April 26, 2024 14:21
Copy link
Contributor

@hermeGarcia hermeGarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one thing to look at!

nucliadb_node/src/grpc/grpc_writer.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jotare jotare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

nucliadb_node/src/settings.rs Show resolved Hide resolved
@lferran lferran force-pushed the poc-node-download-resource-from-object-storage branch from 94d083f to d0f1e65 Compare April 30, 2024 11:54
@lferran lferran merged commit f530165 into main May 2, 2024
92 of 94 checks passed
@lferran lferran deleted the poc-node-download-resource-from-object-storage branch May 2, 2024 14:00
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.

None yet

3 participants