Skip to content

🐛 Fixed issue where GC stopping too early #7457

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 1, 2025
Merged

🐛 Fixed issue where GC stopping too early #7457

merged 5 commits into from
Apr 1, 2025

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Mar 31, 2025 •
edited
Loading

What do these changes do?

There where situations in which Step 3 of the GC was never executed, causing sidecars to be accumulated.

  • in case of errors in step 1 and step 2 the GC would stop and not reach step 3 which is very important for removing system resource
  • Instead of continuing the process wen list_node_ids_in_project raised, the process is stopped. This could cause the GC to stop when services still required removal. (Issue introduced by 🐛 Avoid GC form detecting services as orphans when they are not #7354)

Related issue/s

How to test

Dev-ops checklist

@GitHK GitHK self-assigned this Mar 31, 2025
@GitHK GitHK added this to the The Awakening milestone Mar 31, 2025
@GitHK GitHK added t:enhancement Improvement or request on an existing feature a:webserver webserver's codebase. Assigning the area is particularly useful for bugs labels Mar 31, 2025
@GitHK GitHK changed the title 🐛 Fixed issue where GC stopping with list_node_ids_in_project raised 🐛 Fixed issue where GC stopping with list_node_ids_in_project raised Mar 31, 2025
@codecov
Copy link

codecov bot commented Mar 31, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.11%. Comparing base (f09b9c2) to head (96e9d37).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #7457      +/-   ##
==========================================
- Coverage   87.39%   84.11%   -3.29%     
==========================================
  Files        1728      668    -1060     
  Lines       66821    31747   -35074     
  Branches     1133      168     -965     
==========================================
- Hits        58401    26704   -31697     
+ Misses       8099     4985    -3114     
+ Partials      321       58     -263
Flag Coverage Δ
integrationtests 65.17% <20.00%> (+0.01%) ⬆️
unittests 87.28% <88.00%> (+0.71%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.30% <ø> (-8.16%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.54% <ø> (-12.77%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 89.08% <ø> (-1.04%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 85.84% <88.00%> (-0.01%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f09b9c2...96e9d37. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GitHK GitHK changed the title 🐛 Fixed issue where GC stopping with list_node_ids_in_project raised 🐛 Fixed issue where GC stopping too early Mar 31, 2025
@GitHK GitHK marked this pull request as ready for review March 31, 2025 13:40
sanderegg
sanderegg approved these changes Mar 31, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

thanks

pcrespov
pcrespov approved these changes Mar 31, 2025
View reviewed changes
@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label Apr 1, 2025
@GitHK
Copy link
Contributor Author

GitHK commented Apr 1, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Apr 1, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d8c8f6c

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 1, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@mergify mergify bot merged commit d8c8f6c into ITISFoundation:master Apr 1, 2025
92 of 95 checks passed
This was referenced Apr 10, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcrespov pcrespov pcrespov approved these changes

@sanderegg sanderegg sanderegg approved these changes

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@giancarloromeo giancarloromeo Awaiting requested review from giancarloromeo

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

Assignees

@GitHK GitHK

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:enhancement Improvement or request on an existing feature

Projects

None yet

Milestone

The Awakening

Development

Successfully merging this pull request may close these issues.

4 participants

@GitHK @pcrespov @sanderegg @matusdrobuliak66