From cd2b04ced88401c3c7290d7fb04ea4e3a7577db5 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 10:47:22 +0000 Subject: [PATCH 01/18] start trying to work out which test causes our one to be slow --- .github/workflows/run-docker-compose-test-on-pr.yml | 2 +- s/pr-check | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100755 s/pr-check diff --git a/.github/workflows/run-docker-compose-test-on-pr.yml b/.github/workflows/run-docker-compose-test-on-pr.yml index 0fd784688..a08073e81 100644 --- a/.github/workflows/run-docker-compose-test-on-pr.yml +++ b/.github/workflows/run-docker-compose-test-on-pr.yml @@ -36,7 +36,7 @@ jobs: - name: Run tests run: | - docker compose exec django pytest -v + s/pr-check diff --git a/s/pr-check b/s/pr-check new file mode 100755 index 000000000..61f996bbf --- /dev/null +++ b/s/pr-check @@ -0,0 +1,3 @@ +#!/bin/bash -e + +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_get_filtered_cases_queryset_for.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py From 94d9400aa4e6688737e45d8a44abd474e979345d Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 10:59:33 +0000 Subject: [PATCH 02/18] try without test_get_filtered_cases_queryset_for.py --- s/pr-check | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/s/pr-check b/s/pr-check index 61f996bbf..d240d1209 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,3 +1,5 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_get_filtered_cases_queryset_for.py epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py From d7cf5f47e0b5c744d0ec937a5eddce51941d667e Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 11:17:39 +0000 Subject: [PATCH 03/18] test without test_cases_aggregated_by_attribute --- s/pr-check | 1 - 1 file changed, 1 deletion(-) diff --git a/s/pr-check b/s/pr-check index d240d1209..3a36e9889 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,5 +1,4 @@ #!/bin/bash -e s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ - epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py From aff92b889d9abfddbc7949e712ddffb0ed377296 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 11:32:35 +0000 Subject: [PATCH 04/18] test it is definitely test_calculate_kpi_value_counts_queryset --- s/pr-check | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/s/pr-check b/s/pr-check index 3a36e9889..ab1082c3e 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,4 +1,5 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_get_filtered_cases_queryset_for.py \ epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py From 80aed20157a942dfe45df037e17492220e7402d9 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 11:43:55 +0000 Subject: [PATCH 05/18] comment out code in test_calculate_kpi_value_counts_queryset until we work out what is making the other test upset --- ...est_calculate_kpi_value_counts_queryset.py | 48 +++++++++---------- s/pr-check | 3 +- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index 2e4eb57ef..69a1cfb53 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -110,28 +110,28 @@ def test_calculate_kpi_value_counts_queryset_all_levels( cohort=6, ) - value_counts = calculate_kpi_value_counts_queryset( - filtered_cases=filtered_cases, - abstraction_level=abstraction_level, - kpis=[ - "ecg", - "mental_health_support", - ], - ) + # value_counts = calculate_kpi_value_counts_queryset( + # filtered_cases=filtered_cases, + # abstraction_level=abstraction_level, + # kpis=[ + # "ecg", + # "mental_health_support", + # ], + # ) - if abstraction_level is EnumAbstractionLevel.NATIONAL: - expected_scores = { - "ecg_passed": 20, - "ecg_total_eligible": 40, - "ecg_ineligible": 20, - "ecg_incomplete": 20, - "mental_health_support_passed": 20, - "mental_health_support_total_eligible": 40, - "mental_health_support_ineligible": 20, - "mental_health_support_incomplete": 20, - } - assert value_counts == expected_scores - else: - for vc in value_counts: - abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") - assert vc == expected_scores[abstraction_code] + # if abstraction_level is EnumAbstractionLevel.NATIONAL: + # expected_scores = { + # "ecg_passed": 20, + # "ecg_total_eligible": 40, + # "ecg_ineligible": 20, + # "ecg_incomplete": 20, + # "mental_health_support_passed": 20, + # "mental_health_support_total_eligible": 40, + # "mental_health_support_ineligible": 20, + # "mental_health_support_incomplete": 20, + # } + # assert value_counts == expected_scores + # else: + # for vc in value_counts: + # abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") + # assert vc == expected_scores[abstraction_code] diff --git a/s/pr-check b/s/pr-check index ab1082c3e..8387cc33d 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,5 +1,6 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_get_filtered_cases_queryset_for.py \ epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py From 302f5646901d32e35c208129b836bc138aa9ba05 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 11:58:40 +0000 Subject: [PATCH 06/18] comment out some more code can you believe I got a bachelors degree in computer science and this is the best debugging idea I could come up with --- .../test_calculate_kpi_value_counts_queryset.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index 69a1cfb53..2137c9e63 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -104,11 +104,11 @@ def test_calculate_kpi_value_counts_queryset_all_levels( for code in ods_codes: organisation = Organisation.objects.get(ods_code=code) - filtered_cases = get_filtered_cases_queryset_for( - organisation=organisation, - abstraction_level=abstraction_level, - cohort=6, - ) + # filtered_cases = get_filtered_cases_queryset_for( + # organisation=organisation, + # abstraction_level=abstraction_level, + # cohort=6, + # ) # value_counts = calculate_kpi_value_counts_queryset( # filtered_cases=filtered_cases, From 3e3b93a183209d6217dfb31efbde1c6be5a84f8e Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 12:10:38 +0000 Subject: [PATCH 07/18] is it _register_kpi_scored_cases? --- ...est_calculate_kpi_value_counts_queryset.py | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index 2137c9e63..96e58275e 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -86,52 +86,52 @@ def test_calculate_kpi_value_counts_queryset_all_levels( } expected_scores = {code: kpi_scores_expected for code in abstraction_codes} - _register_kpi_scored_cases( - e12_case_factory, - ods_codes=ods_codes, - num_cases=( - 5 - if abstraction_level - not in [ - # in these 3 abstraction levels, kids are split into half the number of organisations, so register double the nubmer of kids (10) instead of 5 - EnumAbstractionLevel.ORGANISATION, - EnumAbstractionLevel.TRUST, - ] - else 10 - ), - ) + # _register_kpi_scored_cases( + # e12_case_factory, + # ods_codes=ods_codes, + # num_cases=( + # 5 + # if abstraction_level + # not in [ + # # in these 3 abstraction levels, kids are split into half the number of organisations, so register double the nubmer of kids (10) instead of 5 + # EnumAbstractionLevel.ORGANISATION, + # EnumAbstractionLevel.TRUST, + # ] + # else 10 + # ), + # ) - for code in ods_codes: - organisation = Organisation.objects.get(ods_code=code) + # for code in ods_codes: + # organisation = Organisation.objects.get(ods_code=code) - # filtered_cases = get_filtered_cases_queryset_for( - # organisation=organisation, - # abstraction_level=abstraction_level, - # cohort=6, - # ) + # filtered_cases = get_filtered_cases_queryset_for( + # organisation=organisation, + # abstraction_level=abstraction_level, + # cohort=6, + # ) - # value_counts = calculate_kpi_value_counts_queryset( - # filtered_cases=filtered_cases, - # abstraction_level=abstraction_level, - # kpis=[ - # "ecg", - # "mental_health_support", - # ], - # ) + # value_counts = calculate_kpi_value_counts_queryset( + # filtered_cases=filtered_cases, + # abstraction_level=abstraction_level, + # kpis=[ + # "ecg", + # "mental_health_support", + # ], + # ) - # if abstraction_level is EnumAbstractionLevel.NATIONAL: - # expected_scores = { - # "ecg_passed": 20, - # "ecg_total_eligible": 40, - # "ecg_ineligible": 20, - # "ecg_incomplete": 20, - # "mental_health_support_passed": 20, - # "mental_health_support_total_eligible": 40, - # "mental_health_support_ineligible": 20, - # "mental_health_support_incomplete": 20, - # } - # assert value_counts == expected_scores - # else: - # for vc in value_counts: - # abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") - # assert vc == expected_scores[abstraction_code] + # if abstraction_level is EnumAbstractionLevel.NATIONAL: + # expected_scores = { + # "ecg_passed": 20, + # "ecg_total_eligible": 40, + # "ecg_ineligible": 20, + # "ecg_incomplete": 20, + # "mental_health_support_passed": 20, + # "mental_health_support_total_eligible": 40, + # "mental_health_support_ineligible": 20, + # "mental_health_support_incomplete": 20, + # } + # assert value_counts == expected_scores + # else: + # for vc in value_counts: + # abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") + # assert vc == expected_scores[abstraction_code] From 7b6aafe16a0008f8ef106891d71ccf8500e3b7da Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 12:22:16 +0000 Subject: [PATCH 08/18] is it the call to calculate_kpis? --- .../aggregate_by_tests/helpers.py | 5 +- ...est_calculate_kpi_value_counts_queryset.py | 91 ++++++++++--------- 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py index df6268a05..1034a3339 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py @@ -34,7 +34,7 @@ def _register_cases_in_organisation( def _register_kpi_scored_cases( - e12_case_factory, ods_codes: "list[str]", num_cases: int = 10 + e12_case_factory, ods_codes: "list[str]", num_cases: int = 10, debug_calculate_kpis=True ): """Helper function to return a queryset of num_cases kids with scored, known KPI scores. @@ -112,7 +112,8 @@ def _register_kpi_scored_cases( test_case.registration.audit_progress.investigations_complete = True test_case.registration.audit_progress.management_complete = True - calculate_kpis(registration_instance=test_case.registration) + if debug_calculate_kpis: + calculate_kpis(registration_instance=test_case.registration) def _clean_cases_from_test_db() -> None: diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index 96e58275e..93ba59728 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -86,52 +86,53 @@ def test_calculate_kpi_value_counts_queryset_all_levels( } expected_scores = {code: kpi_scores_expected for code in abstraction_codes} - # _register_kpi_scored_cases( - # e12_case_factory, - # ods_codes=ods_codes, - # num_cases=( - # 5 - # if abstraction_level - # not in [ - # # in these 3 abstraction levels, kids are split into half the number of organisations, so register double the nubmer of kids (10) instead of 5 - # EnumAbstractionLevel.ORGANISATION, - # EnumAbstractionLevel.TRUST, - # ] - # else 10 - # ), - # ) + _register_kpi_scored_cases( + e12_case_factory, + ods_codes=ods_codes, + num_cases=( + 5 + if abstraction_level + not in [ + # in these 3 abstraction levels, kids are split into half the number of organisations, so register double the nubmer of kids (10) instead of 5 + EnumAbstractionLevel.ORGANISATION, + EnumAbstractionLevel.TRUST, + ] + else 10 + ), + debug_calculate_kpis=False, + ) - # for code in ods_codes: - # organisation = Organisation.objects.get(ods_code=code) + for code in ods_codes: + organisation = Organisation.objects.get(ods_code=code) - # filtered_cases = get_filtered_cases_queryset_for( - # organisation=organisation, - # abstraction_level=abstraction_level, - # cohort=6, - # ) + filtered_cases = get_filtered_cases_queryset_for( + organisation=organisation, + abstraction_level=abstraction_level, + cohort=6, + ) - # value_counts = calculate_kpi_value_counts_queryset( - # filtered_cases=filtered_cases, - # abstraction_level=abstraction_level, - # kpis=[ - # "ecg", - # "mental_health_support", - # ], - # ) + value_counts = calculate_kpi_value_counts_queryset( + filtered_cases=filtered_cases, + abstraction_level=abstraction_level, + kpis=[ + "ecg", + "mental_health_support", + ], + ) - # if abstraction_level is EnumAbstractionLevel.NATIONAL: - # expected_scores = { - # "ecg_passed": 20, - # "ecg_total_eligible": 40, - # "ecg_ineligible": 20, - # "ecg_incomplete": 20, - # "mental_health_support_passed": 20, - # "mental_health_support_total_eligible": 40, - # "mental_health_support_ineligible": 20, - # "mental_health_support_incomplete": 20, - # } - # assert value_counts == expected_scores - # else: - # for vc in value_counts: - # abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") - # assert vc == expected_scores[abstraction_code] + if abstraction_level is EnumAbstractionLevel.NATIONAL: + expected_scores = { + "ecg_passed": 20, + "ecg_total_eligible": 40, + "ecg_ineligible": 20, + "ecg_incomplete": 20, + "mental_health_support_passed": 20, + "mental_health_support_total_eligible": 40, + "mental_health_support_ineligible": 20, + "mental_health_support_incomplete": 20, + } + assert value_counts == expected_scores + else: + for vc in value_counts: + abstraction_code = vc.pop(f"organisation__{abstraction_level.value}") + assert vc == expected_scores[abstraction_code] From 5853a04a555b7935d0defe51265e38f2b941fe12 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 12:33:11 +0000 Subject: [PATCH 09/18] is it creating the cases? --- .../aggregate_by_tests/helpers.py | 20 +++++++++---------- ...est_calculate_kpi_value_counts_queryset.py | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py index 1034a3339..11f5af149 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/helpers.py @@ -34,7 +34,7 @@ def _register_cases_in_organisation( def _register_kpi_scored_cases( - e12_case_factory, ods_codes: "list[str]", num_cases: int = 10, debug_calculate_kpis=True + e12_case_factory, ods_codes: "list[str]", num_cases: int = 10, debug_create_cases: bool = True ): """Helper function to return a queryset of num_cases kids with scored, known KPI scores. @@ -89,13 +89,14 @@ def _register_kpi_scored_cases( ineligible_answers, incomplete_answers, ]: - test_cases = e12_case_factory.create_batch( - num_cases, - organisations__organisation=organisation, - first_name=f"temp_{organisation.name}", - **answer_set, - ) - filled_case_objects += test_cases + if debug_create_cases: + test_cases = e12_case_factory.create_batch( + num_cases, + organisations__organisation=organisation, + first_name=f"temp_{organisation.name}", + **answer_set, + ) + filled_case_objects += test_cases for test_case in filled_case_objects: test_case.registration.completed_first_year_of_care_date = ( @@ -112,8 +113,7 @@ def _register_kpi_scored_cases( test_case.registration.audit_progress.investigations_complete = True test_case.registration.audit_progress.management_complete = True - if debug_calculate_kpis: - calculate_kpis(registration_instance=test_case.registration) + calculate_kpis(registration_instance=test_case.registration) def _clean_cases_from_test_db() -> None: diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index 93ba59728..f837fb15d 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -99,7 +99,7 @@ def test_calculate_kpi_value_counts_queryset_all_levels( ] else 10 ), - debug_calculate_kpis=False, + debug_create_cases=False, ) for code in ods_codes: From 1f2f772b3351722e3dc672309fb7a2d95bf074b0 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 13:02:35 +0000 Subject: [PATCH 10/18] Try them as two separate runs This is to try and work out if the slowness is inherent to what is left in the test database itself or caused by running both of them together --- .../test_calculate_kpi_value_counts_queryset.py | 2 +- s/pr-check | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py index f837fb15d..88b72c649 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py @@ -99,7 +99,7 @@ def test_calculate_kpi_value_counts_queryset_all_levels( ] else 10 ), - debug_create_cases=False, + debug_create_cases=True, ) for code in ods_codes: diff --git a/s/pr-check b/s/pr-check index 8387cc33d..f72e973ea 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,6 +1,4 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ - epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_cases_aggregated_by_attribute.py \ - epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_get_filtered_cases_queryset_for.py \ - epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From 0facb3526b6948b153f4726a417ecd071ace6fa1 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 13:16:13 +0000 Subject: [PATCH 11/18] try to see if analyze ever did actually make a difference --- s/pr-check | 1 + 1 file changed, 1 insertion(+) diff --git a/s/pr-check b/s/pr-check index f72e973ea..6155c91b0 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,4 +1,5 @@ #!/bin/bash -e s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py +docker compose exec postgis psql -U epilepsy12user -d test_epilepsy12db -c 'analyze;' s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From 7c8a950996eae96883f7795c4309e3934e96f487 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 14:25:31 +0000 Subject: [PATCH 12/18] as an experiment disable nested loops --- docker-compose.yml | 1 + s/pr-check | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index da30aa84e..32f0ed8f7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,7 @@ services: volumes: - postgis-data:/var/lib/postgresql/data restart: always + command: postgres -c enable_nestloop=off mkdocs: <<: *global # this will inherit config from x-global-environment diff --git a/s/pr-check b/s/pr-check index 6155c91b0..5638ab95c 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,5 +1,4 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py -docker compose exec postgis psql -U epilepsy12user -d test_epilepsy12db -c 'analyze;' -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file +s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From 28bead7da9753d2ce0cbb68a77bb679ec383edd5 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 19:14:43 +0000 Subject: [PATCH 13/18] On a hunch try disabling the genetic query planner --- docker-compose.yml | 2 +- epilepsy12/common_view_functions/aggregate_by.py | 4 ++++ s/pr-check | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 32f0ed8f7..aa63aa906 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,7 +41,7 @@ services: volumes: - postgis-data:/var/lib/postgresql/data restart: always - command: postgres -c enable_nestloop=off + command: postgres -c geqo=off mkdocs: <<: *global # this will inherit config from x-global-environment diff --git a/epilepsy12/common_view_functions/aggregate_by.py b/epilepsy12/common_view_functions/aggregate_by.py index 477e87f8c..d670b8d2c 100644 --- a/epilepsy12/common_view_functions/aggregate_by.py +++ b/epilepsy12/common_view_functions/aggregate_by.py @@ -403,6 +403,10 @@ def update_kpi_aggregation_model( return + print(f"!! kpi_value_counts query !!") + print(f"!! {kpi_value_counts.query} !!") + print(f"!! {kpi_value_counts.explain()}") + # update models where numbers have changed. for value_count in kpi_value_counts: ABSTRACTION_CODE = value_count.pop(f"organisation__{abstraction_level.value}") diff --git a/s/pr-check b/s/pr-check index 5638ab95c..6ffe8b77f 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,4 +1,4 @@ #!/bin/bash -e -s/test epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ +s/test -s epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From a3a41b0b3eec927937b48775cbb6c0ba3eb16340 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 19:34:15 +0000 Subject: [PATCH 14/18] try disabling nested loops just for the bad test --- docker-compose.yml | 1 - .../test_update_kpiaggregation_model.py | 13 ++++++++++++- s/pr-check | 6 ++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index aa63aa906..da30aa84e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,7 +41,6 @@ services: volumes: - postgis-data:/var/lib/postgresql/data restart: always - command: postgres -c geqo=off mkdocs: <<: *global # this will inherit config from x-global-environment diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py index d66784cac..2c84313e6 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py @@ -3,6 +3,7 @@ # 3rd party imports from django.apps import apps +from django.db import connection # E12 imports from epilepsy12.common_view_functions import ( @@ -21,6 +22,16 @@ from .helpers import _clean_cases_from_test_db, _register_kpi_scored_cases +@pytest.fixture +def disable_nested_loops_in_db(db): + cursor = connection.cursor() + print("!! Disabling nested loops in DB") + cursor.execute("SET enable_nestloop TO off") + yield + print("!! Re-enabling nested loops in DB") + cursor.execute("SET enable_nestloop TO on") + + @pytest.mark.parametrize( "abstraction_level, abstraction_codes, ods_codes", [ @@ -68,7 +79,7 @@ ) @pytest.mark.django_db def test_update_kpi_aggregation_model_all_levels( - abstraction_level, abstraction_codes, ods_codes, e12_case_factory + abstraction_level, abstraction_codes, ods_codes, e12_case_factory, disable_nested_loops_in_db ): """Testing the `update_kpi_aggregation_model` fn. diff --git a/s/pr-check b/s/pr-check index 6ffe8b77f..64681a2ef 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,4 +1,6 @@ #!/bin/bash -e -s/test -s epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ - epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file +s/test + +# s/test -s epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ +# epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From 3e481a5aa116462715fc7472b21b6084069e0809 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 19:46:54 +0000 Subject: [PATCH 15/18] Run explain with analyze and buffers --- epilepsy12/common_view_functions/aggregate_by.py | 2 +- .../test_update_kpiaggregation_model.py | 13 +++++++------ s/pr-check | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/epilepsy12/common_view_functions/aggregate_by.py b/epilepsy12/common_view_functions/aggregate_by.py index d670b8d2c..e3e7ca9c8 100644 --- a/epilepsy12/common_view_functions/aggregate_by.py +++ b/epilepsy12/common_view_functions/aggregate_by.py @@ -405,7 +405,7 @@ def update_kpi_aggregation_model( print(f"!! kpi_value_counts query !!") print(f"!! {kpi_value_counts.query} !!") - print(f"!! {kpi_value_counts.explain()}") + print(f"!! {kpi_value_counts.explain(analyze=True, buffers=True)}") # update models where numbers have changed. for value_count in kpi_value_counts: diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py index 2c84313e6..68464cdba 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py @@ -24,12 +24,13 @@ @pytest.fixture def disable_nested_loops_in_db(db): - cursor = connection.cursor() - print("!! Disabling nested loops in DB") - cursor.execute("SET enable_nestloop TO off") - yield - print("!! Re-enabling nested loops in DB") - cursor.execute("SET enable_nestloop TO on") + pass + # cursor = connection.cursor() + # print("!! Disabling nested loops in DB") + # cursor.execute("SET enable_nestloop TO off") + # yield + # print("!! Re-enabling nested loops in DB") + # cursor.execute("SET enable_nestloop TO on") @pytest.mark.parametrize( diff --git a/s/pr-check b/s/pr-check index 64681a2ef..db6dc89c5 100755 --- a/s/pr-check +++ b/s/pr-check @@ -1,6 +1,6 @@ #!/bin/bash -e -s/test +# s/test -# s/test -s epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ -# epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file +s/test -s epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_calculate_kpi_value_counts_queryset.py \ + epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py \ No newline at end of file From c08c54bf1e94696eaa81abfc279a7554ff6a311f Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 20:22:52 +0000 Subject: [PATCH 16/18] what query plan is it used with nested loops turned off? --- .../test_update_kpiaggregation_model.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py index 68464cdba..8bdd1df50 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py @@ -24,13 +24,13 @@ @pytest.fixture def disable_nested_loops_in_db(db): - pass - # cursor = connection.cursor() - # print("!! Disabling nested loops in DB") - # cursor.execute("SET enable_nestloop TO off") - # yield - # print("!! Re-enabling nested loops in DB") - # cursor.execute("SET enable_nestloop TO on") + # pass + cursor = connection.cursor() + print("!! Disabling nested loops in DB") + cursor.execute("SET enable_nestloop TO off") + yield + print("!! Re-enabling nested loops in DB") + cursor.execute("SET enable_nestloop TO on") @pytest.mark.parametrize( From c8aa56ab6481a27e31a47475c3ab062f201ac134 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 20:45:03 +0000 Subject: [PATCH 17/18] Try postgres 16 --- docker-compose.yml | 2 +- .../test_update_kpiaggregation_model.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index da30aa84e..3e877b510 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -37,7 +37,7 @@ services: # PostgreSQL with PostGIS extension postgis: <<: *global # this will inherit config from x-global-environment - image: postgis/postgis:15-3.3 + image: postgis/postgis:16-3.5 volumes: - postgis-data:/var/lib/postgresql/data restart: always diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py index 8bdd1df50..68464cdba 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py @@ -24,13 +24,13 @@ @pytest.fixture def disable_nested_loops_in_db(db): - # pass - cursor = connection.cursor() - print("!! Disabling nested loops in DB") - cursor.execute("SET enable_nestloop TO off") - yield - print("!! Re-enabling nested loops in DB") - cursor.execute("SET enable_nestloop TO on") + pass + # cursor = connection.cursor() + # print("!! Disabling nested loops in DB") + # cursor.execute("SET enable_nestloop TO off") + # yield + # print("!! Re-enabling nested loops in DB") + # cursor.execute("SET enable_nestloop TO on") @pytest.mark.parametrize( From f77bcd7c2c48b7017880afe92e0415a59311263f Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Tue, 17 Dec 2024 21:00:47 +0000 Subject: [PATCH 18/18] Maybe it was the presorted aggregate wot dun it? --- .../test_update_kpiaggregation_model.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py index 68464cdba..312208815 100644 --- a/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py +++ b/epilepsy12/tests/common_view_functions_tests/aggregate_by_tests/test_update_kpiaggregation_model.py @@ -23,14 +23,14 @@ @pytest.fixture -def disable_nested_loops_in_db(db): - pass - # cursor = connection.cursor() - # print("!! Disabling nested loops in DB") - # cursor.execute("SET enable_nestloop TO off") - # yield - # print("!! Re-enabling nested loops in DB") - # cursor.execute("SET enable_nestloop TO on") +def disable_presorted_aggregate_in_db(db): + # pass + cursor = connection.cursor() + print("!! Disabling presorted aggregate in DB") + cursor.execute("SET enable_presorted_aggregate TO off") + yield + print("!! Re-enabling presorted aggregate in DB") + cursor.execute("SET enable_presorted_aggregate TO on") @pytest.mark.parametrize( @@ -80,7 +80,7 @@ def disable_nested_loops_in_db(db): ) @pytest.mark.django_db def test_update_kpi_aggregation_model_all_levels( - abstraction_level, abstraction_codes, ods_codes, e12_case_factory, disable_nested_loops_in_db + abstraction_level, abstraction_codes, ods_codes, e12_case_factory, disable_presorted_aggregate_in_db ): """Testing the `update_kpi_aggregation_model` fn.