switch back from GIST to GIN indexes #2769
Conversation
synapse/storage/search.py
Outdated
| def _background_reindex_gin_search(self, progress, batch_size): | ||
| '''This handles old synapses which used GIST indexes; converting them | ||
| back to be GIN as per the actual schema. Otherwise it crashes out | ||
| as a NOOP |
There was a problem hiding this comment.
How does the background reindexer handle things that crash?
There was a problem hiding this comment.
as far as I'm aware, it makes the updater get stuck in a loop, attempting to do the update every second forever.
this... sounds less than ideal.
There was a problem hiding this comment.
ah, but a66f489 was an attempt to fix this.
|
This won't work as a) you don't register the new background update in a new delta file and b) you've removed a delta update function without removing it from the delta files |
synapse/storage/search.py
Outdated
|
|
||
| conn.set_session(autocommit=False) | ||
| conn.set_session(autocommit=False) | ||
| except e: |
People may have queued updates for this, so we can't just delete it.
... as a precursor to making event storing and doing the bg update share some code.
Add some comments and improve exception handling when twiddling work_mem for the search update
we can reuse the same code as is used for event insert, for doing the background index population.
richvdh
force-pushed
the
matthew/hit_the_gin
branch
from
4e10a11 to
83e2d27
Compare
|
@erikjohnston please could you have another look at this? I suggest you start from scratch, rather than attempting to pick out the delta from your previous review. |
We're up to schema v47 on develop now, so this will have to go in there to have an effect. This might cause an error if somebody has already run it in the v46 guise, and runs it again in the v47 guise, because it will cause a duplicate entry in the bbackground_updates table. On the other hand, the entry is removed once it is complete, and it is unlikely that anyone other than matrix.org has run it on v46. The update itself is harmless to re-run because it deliberately copes with the index already existing.
richvdh
force-pushed
the
matthew/hit_the_gin
branch
from
8b8dab7 to
ddb6a79
Compare
synapse/storage/search.py
Outdated
| txn.executemany(sql, args) | ||
| except Exception: | ||
| # we need to reset work_mem, but doing so may throw a new | ||
| # exception and we want to preserve the original |
There was a problem hiding this comment.
Do we? work_mem should be reset if the transaction aborts: https://www.postgresql.org/docs/9.4/static/sql-set.html
fixes #2753