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

[bug fix] Fix check_synapse_cache_size function; allow file size to be float #1389

Merged
merged 11 commits into from
Apr 5, 2024

Conversation

linglp
Copy link
Contributor

@linglp linglp commented Mar 20, 2024

Context

We encountered this error on AWS side (see Jira ticket here) This error was first caught by running schematic profiler, but it is now affecting a lot of endpoints, including manifest/generate, model/submit, /storage/project/datasets and so on on dev instance. The same bug also appears in staging and prod. The only reason that we are not receiving reports about this error from staging/prod is most likely because the cache size has not yet reached "GB" level

Problem

convert_gb_to_bytes has wrong typing, and this line: size_in_gb = float(size.rstrip("G")) was changed to size_in_gb = int(size.rstrip("G")) which caused the problem. Test also didn't catch this error because it was testing situations where cache size is XXKB, but not when cache size is XXGB.

Solution

  1. I removed convert_gb_to_bytes function from the code base. The function was doing a simple calculation that I think we don't need a function to do it. This also resolve the "wrong typing" issue of the function since this function no longer exists.
  2. The previous test_check_synapse_cache_size function creates a small test file to test check_synapse_cache_size. This means that the test was only testing when the cache size is "KB". I updated the test so that test files with various sizes (KB, MB, GB) are all tested.

Test

  1. Make sure all the current tests are working on GH.
  2. I ran schematic profiler while monitoring AWS logs. I could see purging cache is working:
Screen Shot 2024-03-21 at 6 20 23 PM 3) Make sure all the tests are returning 200 on schematic profiler: https://www.synapse.org/#!Synapse:syn51385540/tables/query/eyJzcWwiOiJTRUxFQ1QgKiBGUk9NIHN5bjUxMzg1NTQwIiwgImluY2x1ZGVFbnRpdHlFdGFnIjp0cnVlLCAib2Zmc2V0Ijo1NzUsICJsaW1pdCI6MjV9

@mialy-defelice
Copy link
Contributor

@linglp can you put the ticket number in the branch name so its tracked?

Copy link

sonarqubecloud bot commented Apr 4, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@linglp linglp merged commit f91c4ed into develop Apr 5, 2024
4 checks passed
@linglp linglp deleted the develop-fix-cache branch April 5, 2024 17:20
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