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

[23.1] Fix disk usage recalculation for distributed object stores #16380

Merged

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Jul 7, 2023

Found this while adding the additional test cases in the first commit. I don't know yet why this wasn't failing on usegalaxy.org (maybe an empty iterable in quota_source_map.ids_per_quota_source() ?), and I suppose it won't fix the issue that @natefoo found in the backend channel.

fd15b82 fixes a bind param mismatch. I'll say the unpopular thing here: we should've used sqlalchemy core for this, since that'd have done the parameter binding for us. Found this while adding the additional test cases in the first commit

53dc916f0d722a350c53dba70e4fabda42d2d7ff fixes the case Nate reported in the backend channel, which is because main uses a hierarchical object store where no backend defines a quota_source attribute.

Disk usage calculation seems to be broken on .org, does anyone know of recent changes that might be suspect?
e.g.:

galaxy_main=> select sum(d.total_size) from history_dataset_association hda join history h on hda.history_id = h.id join galaxy_user u on h.user_id = u.id join dataset d on hda.dataset_id = d.id where u.username = '84756234z' and not hda.purged and not d.purged;
     sum
--------------
 320534890258
(1 row)

galaxy_main=> select disk_usage from galaxy_user where username = '84756234z';
 disk_usage
------------
          0

And recalculating just remains 0.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

to include a distributed object store config with and without celery
enabled.
@mvdbeek mvdbeek changed the title [23.1] Fix calculate_user_disk_usage_statements with disributed object stores [23.1] Fix calculate_user_disk_usage_statements with distributed object stores Jul 7, 2023
@github-actions github-actions bot added this to the 23.2 milestone Jul 7, 2023
@mvdbeek

This comment was marked as outdated.

@mvdbeek mvdbeek force-pushed the object_store_fix_quota_label branch from 6218c3a to 85284d3 Compare July 7, 2023 12:22
@mvdbeek mvdbeek changed the title [23.1] Fix calculate_user_disk_usage_statements with distributed object stores [23.1] Fix disk usage recalculation for distributed object stores Jul 7, 2023
@mvdbeek mvdbeek force-pushed the object_store_fix_quota_label branch from 85284d3 to 53dc916 Compare July 7, 2023 12:42
@mvdbeek
Copy link
Member Author

mvdbeek commented Jul 7, 2023

API test failures are unrelated and due to galaxy-web-05 being down.

Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fix and the extended coverage!

The remaining integration test is taking a surprisingly long time to finish but I guess also unrelated to this fix.

@mvdbeek
Copy link
Member Author

mvdbeek commented Jul 7, 2023

Yep, it took over an hour to run the node setup action:

Screenshot 2023-07-08 at 00 52 40

Copy link
Member

@jdavcs jdavcs left a comment

Choose a reason for hiding this comment

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

Looks right to me. Also, a very enthusiastic +1 on the "unpopular" mention of using SQLAlchemy Core instead of raw SQL, especially for parametrized statements.

@mvdbeek mvdbeek merged commit de40c46 into galaxyproject:release_23.1 Jul 7, 2023
36 of 37 checks passed
@martenson martenson deleted the object_store_fix_quota_label branch July 9, 2023 18:07
@dannon dannon modified the milestones: 23.2, 23.1 Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants