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

thread dump service rust #1270

Merged
merged 20 commits into from
Sep 6, 2023
Merged

thread dump service rust #1270

merged 20 commits into from
Sep 6, 2023

Conversation

hermeGarcia
Copy link
Contributor

@hermeGarcia hermeGarcia commented Aug 29, 2023

Description

Adds the /__dump endpoint to the rust HTTP service to help debugging.

How was this PR tested?

Describe how you tested this PR.

@hermeGarcia hermeGarcia requested a review from a team as a code owner August 29, 2023 14:55
@hermeGarcia hermeGarcia reopened this Aug 29, 2023
@hermeGarcia hermeGarcia marked this pull request as draft August 29, 2023 14:57
@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d6cb139) 85.02% compared to head (f6a304a) 85.02%.

❗ Current head f6a304a differs from pull request most recent head 88848ba. Consider uploading reports for the commit 88848ba to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1270   +/-   ##
=======================================
  Coverage   85.02%   85.02%           
=======================================
  Files         412      412           
  Lines       22811    22811           
=======================================
  Hits        19396    19396           
  Misses       3415     3415           
Flag Coverage Δ
ingest 74.91% <ø> (ø)
nucliadb 65.89% <ø> (ø)
reader 74.08% <ø> (ø)
sdk 42.51% <ø> (ø)
search 83.96% <ø> (+<0.01%) ⬆️
standalone 82.97% <ø> (ø)
train 60.25% <ø> (ø)
utils 84.63% <ø> (ø)
writer 84.94% <ø> (ø)

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.

@tarekziade tarekziade self-assigned this Sep 4, 2023
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.

license-eye has totally checked 840 files.

Valid Invalid Ignored Fixed
727 1 112 0
Click to see the invalid file list
  • nucliadb_node/pyproject.toml

nucliadb_node/pyproject.toml Outdated Show resolved Hide resolved
@tarekziade tarekziade changed the title thread dumb service rust thread dump service rust Sep 4, 2023
@tarekziade
Copy link
Contributor

/bench

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

Benchmarks comparison to main branch

Test PR benchmark Main benchmark %
Total Storage Size 14601301 15269195.0 -4.37
Writing Time 764 955.0 -20.00
Reading Time 5071 6829.0 -25.74
Reading Time with Labels 28729 35862.0 -19.89
Labels Index Time 2031 2324.0 -12.61
Labels Search Time 383 357.0 7.28

Happy hacking!

@tarekziade
Copy link
Contributor

/bench

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

Benchmarks comparison to main branch

Test PR benchmark Main benchmark %
Total Storage Size 14605563 15269195.0 -4.35
Writing Time 803 955.0 -15.92
Reading Time 5990 6829.0 -12.29
Reading Time with Labels 29354 35862.0 -18.15
Labels Index Time 1999 2324.0 -13.98
Labels Search Time 370 357.0 3.64

Happy hacking!

@tarekziade tarekziade force-pushed the traces-service branch 2 times, most recently from 4eca9ff to dbc6fbb Compare September 5, 2023 07:42
@tarekziade tarekziade marked this pull request as ready for review September 6, 2023 08:31
@@ -33,13 +34,13 @@ pub struct MetricsServerOptions {
pub default_http_port: u16,
}

pub async fn run_http_metrics_server(options: MetricsServerOptions) {
pub async fn run_http_server(options: MetricsServerOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: you could also rename MetricsServerOptions to ServerOptions for example, as the function is now more generic

Copy link
Contributor

Choose a reason for hiding this comment

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

good idea!

@tarekziade tarekziade added this pull request to the merge queue Sep 6, 2023
Merged via the queue into main with commit 3fcd2a2 Sep 6, 2023
57 checks passed
@tarekziade tarekziade deleted the traces-service branch September 6, 2023 09:07
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