Complement `Adding a push rule wakes up an incremental /sync` is consistently failing

[DMRobertson]

and it's irritating me.

https://github.com/matrix-org/synapse/actions/runs/3472783471/jobs/5804115166 e.g.

From early debugging it looks

• adding the push rule correctly wakes up a user_stream
• the user_stream calls its callback to generate a SyncResult
• we fetch global_account_data and get {} for our troubles
• presumably it is supposed to be a nonempty dict?

[DMRobertson]

"Flakey" might mean that Synapse is broken.

The tests are about a month old (matrix-org/complement#488).

[DMRobertson]

Running a git bisect points the finger at #14376

[DMRobertson]

I'm using COMPLEMENT_DIR=../complement scripts-dev/complement.sh -run 'TestPushSync' to reproduce this.

Edit: or the slightly fancier

SYNAPSE_TEST_LOG_LEVEL=DEBUG COMPLEMENT_DIR=../complement scripts-dev/complement.sh -run 'TestPushSync' -json | gotestfmt | tee test | less

[DMRobertson]

I think that

synapse/synapse/storage/databases/main/account_data.py

Lines 357 to 359 in 36097e8

	changed = self._account_data_stream_cache.has_entity_changed(
	user_id, int(stream_id)
	)

is now False when it should have been True.

[DMRobertson]

According to Erik, this condition

synapse/synapse/storage/util/id_generators.py

Line 256 in 36097e8

if self._is_writer:

should be if not self._is_writer. If I make that change, the test passes (without the revert in #14463). cc @Fizzadar

I'll see if I can hack together a unit test which would detect this.

[Fizzadar]

Aha great spot @DMRobertson! Wild that this wasn't picked up in the tests earlier! EDIT: thank you both!