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

ISD-1516 Remove is_leader check when running db migration #189

Merged
merged 5 commits into from
Feb 20, 2024

Conversation

Thanhphan1147
Copy link
Contributor

@Thanhphan1147 Thanhphan1147 commented Feb 15, 2024

We can take advantage of the idem-potency of the db:migrate task to remove a check and run that task on all units.

Local tests shows a time loss of around 3s per unit for each skipped checks (note that unit-discourse-k8s-1 is doing the actual migration and the other calling the task and skipping it):

unit-discourse-k8s-1: 00:34:47 INFO unit.discourse-k8s/1.juju-log Discourse setup: about to execute migrations
unit-discourse-k8s-1: 00:34:51 INFO unit.discourse-k8s/1.juju-log unit: <ops.model.Unit discourse-k8s/1> DB migration done in 4.5590386390686035 seconds
...
unit-discourse-k8s-0: 00:37:36 INFO unit.discourse-k8s/0.juju-log unit: <ops.model.Unit discourse-k8s/0> DB migration done in 3.47281813621521 seconds
unit-discourse-k8s-0: 00:37:36 INFO unit.discourse-k8s/0.juju-log Discourse setup: about to compile assets

Checklist

@Thanhphan1147 Thanhphan1147 marked this pull request as ready for review February 16, 2024 07:57
@Thanhphan1147 Thanhphan1147 requested a review from a team as a code owner February 16, 2024 07:57
@Thanhphan1147 Thanhphan1147 marked this pull request as draft February 16, 2024 07:57
@Thanhphan1147 Thanhphan1147 marked this pull request as ready for review February 16, 2024 23:51
@Thanhphan1147 Thanhphan1147 changed the title Remove is_leader check when running db migration ISD-1516 Remove is_leader check when running db migration Feb 16, 2024
cbartz
cbartz previously approved these changes Feb 19, 2024
src/charm.py Show resolved Hide resolved
…thub.com:canonical/discourse-k8s-operator into remove_is_leader_check_when_running_db_migration
Copy link
Contributor

Test coverage for cf63c26

Name              Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------
src/charm.py        329     30     80     11    90%   177, 185-186, 350->355, 552-554, 559-560, 571-573, 578-579, 591-593, 598-599, 611-613, 638-640, 682->exit, 692->exit, 722-728, 754->exit, 768-769, 779->exit, 793
src/database.py      30      1      8      1    95%   56
-------------------------------------------------------------
TOTAL               359     31     88     12    90%

Static code analysis report

Run started:2024-02-19 17:59:37.454459

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 1985
  Total lines skipped (#nosec): 1
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@Thanhphan1147 Thanhphan1147 merged commit 9bd7bb8 into main Feb 20, 2024
29 checks passed
@Thanhphan1147 Thanhphan1147 deleted the remove_is_leader_check_when_running_db_migration branch February 20, 2024 11:47
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.

5 participants