Commit graph

5 commits

Author SHA1 Message Date
Matias Bordese
ab700fd4a1
Update OrderedModel serializer not to save previously updated order (#4706)
We are noticing some
[issues](https://ops.grafana-ops.net/goto/sQFLv4XSg?orgId=1) when
updating routes via Terraform because when receiving multiple concurrent
requests updating position, the order is updated in a transaction but it
is then tried to save again in the `.save` call, which could lead to
`IntegrityError`s, because the order may have been updated in a
concurrent request meanwhile.

Test run with the reproduced error (before the serializer update):

```
_____________________________ test_ordered_model_swap_all_to_zero_via_serializer _____________________________

    @pytest.mark.django_db(transaction=True)
    def test_ordered_model_swap_all_to_zero_via_serializer():
        THREADS = 300
        exceptions = []
    
        TestOrderedModel.objects.all().delete()  # clear table
        instances = [TestOrderedModel.objects.create(test_field="test") for _ in range(THREADS)]
    
        # generate random non-unique orders
        random.seed(42)
        positions = [random.randint(0, THREADS - 1) for _ in range(THREADS)]
    
        def update_order_to_zero(idx):
            try:
                instance = instances[idx]
                serializer = TestOrderedModelSerializer(
                    instance, data={"order": 0, "extra_field": idx}, partial=True
                )
                serializer.is_valid(raise_exception=True)
                serializer.save()
                instance.swap(positions[idx])
            except Exception as e:
                exceptions.append(e)
    
        threads = [threading.Thread(target=update_order_to_zero, args=(0,)) for _ in range(THREADS)]
        for thread in threads:
            thread.start()
        for thread in threads:
            thread.join()
    
        # can only check that orders are still sequential and that there are no exceptions
        # can't check the exact order because it changes depending on the order of execution
>       assert not exceptions
E       assert not [IntegrityError(1062, "Duplicate entry 'test-0' for key 'base_testorderedmodel.unique_test_field_order'"), IntegrityEr...rder'"), IntegrityError(1062, "Duplicate entry 'test-0' for key 'base_testorderedmodel.unique_test_field_order'"), ...]

THREADS    = 300
exceptions = [IntegrityError(1062, "Duplicate entry 'test-0' for key 'base_testorderedmodel.unique_test_field_order'"), IntegrityEr...rder'"), IntegrityError(1062, "Duplicate entry 'test-0' for key 'base_testorderedmodel.unique_test_field_order'"), ...]
instances  = [<TestOrderedModel: TestOrderedModel object (841)>, <TestOrderedModel: TestOrderedModel object (842)>, <TestOrderedMod...ject (844)>, <TestOrderedModel: TestOrderedModel object (845)>, <TestOrderedModel: TestOrderedModel object (846)>, ...]
positions  = [57, 12, 140, 125, 114, 71, ...]
thread     = <Thread(Thread-302 (update_order_to_zero), stopped 139636013323840)>
threads    = [<Thread(Thread-3 (update_order_to_zero), stopped 139646722418240)>, <Thread(Thread-4 (update_order_to_zero), stopped ...ate_order_to_zero), stopped 139646608590400)>, <Thread(Thread-8 (update_order_to_zero), stopped 139646600197696)>, ...]
update_order_to_zero = <function test_ordered_model_swap_all_to_zero_via_serializer.<locals>.update_order_to_zero at 0x7f02086274c0>

common/tests/test_ordered_model.py:481: AssertionError
```
2024-07-19 11:56:26 +00:00
Matias Bordese
1a8e4fef40
Update OrderedModel.swap to retry on IntegrityError (#3940)
Related to https://github.com/grafana/oncall-private/issues/2386.

Issue seems related to [multiple concurrent Terraform-triggered
updates](https://ops.grafana-ops.net/explore?schemaVersion=1&panes=%7B%22j0s%22:%7B%22datasource%22:%22000000193%22,%22queries%22:%5B%7B%22refId%22:%22B%22,%22expr%22:%22%7Bcluster%3D%5C%22prod-eu-west-0%5C%22,%20namespace%3D%5C%22amixr-prod%5C%22%7D%20%7C%3D%20%5C%22logger%3Dinsight_logger%20tenant_id%3D735393%5C%22%20%7C%3D%20%5C%22resource_type%3Droute%5C%22%22,%22queryType%22:%22range%22,%22datasource%22:%7B%22type%22:%22loki%22,%22uid%22:%22000000193%22%7D,%22editorMode%22:%22code%22%7D%5D,%22range%22:%7B%22from%22:%221707813893526%22,%22to%22:%221707815856774%22%7D%7D%7D&orgId=1
) changing the route order to zero (and eventually succeeding)

Sample test run with the failing error:
```
========================================== short test summary info ===========================================
FAILED common/tests/test_ordered_model.py::test_ordered_model_swap_all_to_zero - assert not [DoesNotExist(), IntegrityError(1062, "Duplicate entry 'test-0' for key 'base_testorderedmodel...
======================================= 1 failed, 5 warnings in 34.39s =======================================
make: *** [Makefile:283: run-backend-test] Error 1
```
2024-02-22 20:51:05 +00:00
Joey Orlando
d6140cbe8d
Re-enable a few mypy rules + fix existing errors (#2725)
# What this PR does

Related to https://github.com/grafana/oncall/issues/2392

- Re-enable the following `mypy` rules + fix their pre-existing errors:
  - `no-redef`
  - `valid-type`
  - `var-annotated`
- Add stronger return typing to the `GrafanaAPIClient` by use of
generics + add some links to documentation in the method docstrings

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
2023-08-03 09:43:03 +00:00
Vadim Stepanov
dc137d705e
Fix public API integration default route (#2573)
Fix bug related to `order` and default route introduced in
https://github.com/grafana/oncall/pull/2572
2023-07-18 20:29:04 +01:00
Vadim Stepanov
602ed535e3
Fix duplicate orders on routes and escalation policies (#2568)
# What this PR does

Fix duplicate `order` values for models `EscalationPolicy` and
`ChannelFilter` using the same approach as
https://github.com/grafana/oncall/pull/2278.

- Make internal API action `move_to_position` a part of
[OrderedModelViewSet](https://github.com/grafana/oncall/pull/2568/files#diff-eb62521ccbcb072d1bd2156adeadae3b5895bce6d0d54432a23db3948b0ada54R11-R34),
so all ordered model views use the same logic.
- Make public API serializers for ordered models inherit from
[OrderedModelSerializer](https://github.com/grafana/oncall/pull/2568/files#diff-d749f94af5a49adaf5074325cdfad10ddd5a52dbfd78b49582867ebb9c92fae5R6-R38),
so ordered model views are consistent with each other in public API.
- Remove `order` from plugin fronted, since it's not being used
anywhere. The frontend uses step indices & `move_to_position` action
instead.
- Make escalation snapshot use step indices instead of orders, since
orders are not guaranteed to be sequential (+fix a minor off-by-one bug)

## Which issue(s) this PR fixes

https://github.com/grafana/oncall-private/issues/1680

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
2023-07-18 17:17:53 +00:00