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.

Sign up for GitHub

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

Jump to bottom

Fix flakey TestRoomReceiptsReadMarkers #393

Merged
merged 2 commits into from
Jun 13, 2022
Merged

Fix flakey TestRoomReceiptsReadMarkers #393

merged 2 commits into from
Jun 13, 2022

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Jun 13, 2022 •
edited
Loading

We can't do the tests in parallel, as sending a /receipt request after
a /read_markers request on the same event returns an error. (While
/read_markers will return 200 in the reversed situation).

We fix it by splitting the test into two.

Example logs from a failed run:

  synapse_main | 2022-06-10 16:26:54,744 - synapse.http.server - 183 - ERROR - POST-6 - Failed handle request via 'ReceiptRestServlet': <SynapseRequest at 0x7fad10d33d90 method='POST' uri='/_matrix/client/v3/rooms/%21YMYmKAvqozmiNZyrus:hs1/receipt/m.read/$EI-7A_5hLV-7LizT507qIH_E2VUUBQueeIYki4iUib8' clientproto='HTTP/1.0' site='8080'>
  synapse_main | Traceback (most recent call last):
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 366, in _async_render_wrapper
  synapse_main |     callback_return = await self._async_render(request)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/http/server.py", line 572, in _async_render
  synapse_main |     callback_return = await raw_callback_return
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/rest/client/receipts.py", line 79, in on_POST
  synapse_main |     await self.receipts_handler.received_client_receipt(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/handlers/receipts.py", line 162, in received_client_receipt
  synapse_main |     is_new = await self._handle_new_receipts([receipt])
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/handlers/receipts.py", line 112, in _handle_new_receipts
  synapse_main |     res = await self.store.insert_receipt(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/databases/main/receipts.py", line 796, in insert_receipt
  synapse_main |     await self.db_pool.runInteraction(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 835, in runInteraction
  synapse_main |     return await delay_cancellation(_runInteraction())
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/internet/defer.py", line 1656, in _inlineCallbacks
  synapse_main |     result = current_context.run(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/python/failure.py", line 514, in throwExceptionIntoGenerator
  synapse_main |     return g.throw(self.type, self.value, self.tb)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 807, in _runInteraction
  synapse_main |     result = await self.runWithConnection(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 930, in runWithConnection
  synapse_main |     return await make_deferred_yieldable(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/python/threadpool.py", line 244, in inContext
  synapse_main |     result = inContext.theWork()  # type: ignore[attr-defined]
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/python/threadpool.py", line 260, in <lambda>
  synapse_main |     inContext.theWork = lambda: context.call(  # type: ignore[attr-defined]
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/python/context.py", line 117, in callWithContext
  synapse_main |     return self.currentContext().callWithContext(ctx, func, *args, **kw)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/python/context.py", line 82, in callWithContext
  synapse_main |     return func(*args, **kw)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/twisted/enterprise/adbapi.py", line 282, in _runWithConnection
  synapse_main |     result = func(conn, *args, **kw)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 923, in inner_func
  synapse_main |     return func(db_conn, *args, **kwargs)
  2022-06-10 16:26:55,765 WARN received SIGTERM indicating exit request
  2022-06-10 16:26:55,765 INFO waiting for postgres, synapse_main, nginx to die
  2022-06-10 16:26:55,768 INFO stopped: nginx (exit status 0)
  2022-06-10 16:26:55,768 INFO reaped unknown pid 33 (exit status 0)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 671, in new_transaction
  synapse_main |     r = func(cursor, *args, **kwargs)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/databases/main/receipts.py", line 837, in _insert_graph_receipt_txn
  synapse_main |     self.db_pool.simple_insert_txn(
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 1018, in simple_insert_txn
  synapse_main |     txn.execute(sql, vals)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 353, in execute
  synapse_main |     self._do_execute(self.txn.execute, sql, *args)
  synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 395, in _do_execute
  synapse_main |     return func(sql, *args, **kwargs)
  synapse_main | psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "receipts_graph_uniqueness"
  synapse_main | DETAIL:  Key (room_id, receipt_type, user_id)=(!YMYmKAvqozmiNZyrus:hs1, m.read, @alice:hs1) already exists.
  synapse_main | 
  synapse_main | 2022-06-10 16:26:54,753 - synapse.access.http.8080 - 450 - INFO - POST-6 - ::ffff:127.0.0.1 - 8080 - {@alice:hs1} Processed request: 0.041sec/0.005sec (0.005sec, 0.000sec) (0.005sec/0.024sec/4) 67B 500 "POST /_matrix/client/v3/rooms/%21YMYmKAvqozmiNZyrus:hs1/receipt/m.read/$EI-7A_5hLV-7LizT507qIH_E2VUUBQueeIYki4iUib8 HTTP/1.0" "Go-http-client/1.1" [0 dbevts]

@erikjohnston
Fix flakey receipts tests
4762dbe 
We can't do the tests in parallel, as sending a `/receipt` request after
a `/read_markers` request on the same event returns an error. (While
`/read_markers` will return 200 in the reversed situation).
@erikjohnston erikjohnston requested a review from a team June 13, 2022 08:48
@erikjohnston erikjohnston mentioned this pull request Jun 13, 2022
Merged
squahtx
squahtx approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@squahtx squahtx left a comment

Choose a reason for hiding this comment

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

one comment, otherwise lgtm

tests/csapi/apidoc_room_receipts_test.go Outdated
@@ -9,51 +9,51 @@ import (
)

// tests/10apidoc/37room-receipts.pl
func TestRoomReceiptsReadMarkers(t *testing.T) {
func TestRoomReceipts(t *testing.T) {
deployment := Deploy(t, b.BlueprintAlice)
defer deployment.Destroy(t)

alice := deployment.Client(t, "hs1", "@alice:hs1")
roomID := alice.CreateRoom(t, map[string]interface{}{"preset": "public_chat"})

// sytest: POST /rooms/:room_id/receipt can create receipts
Copy link
Contributor

Choose a reason for hiding this comment

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

the doc comments could do with being moved up to before func TestRoomReceipts and func TestRoomMarkers

@squahtx squahtx mentioned this pull request Jun 13, 2022
Merged
4 tasks
@erikjohnston erikjohnston merged commit b676502 into main Jun 13, 2022
@erikjohnston erikjohnston deleted the erikj/receipts_tests branch June 13, 2022 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@squahtx squahtx squahtx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erikjohnston @squahtx