Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix background reindex of origin_server_ts #1145

Merged
merged 1 commit into from
Sep 29, 2016
Merged

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Sep 27, 2016

The storage function _get_events_txn was removed everywhere except from this background reindex. The function was removed due to it being (almost) completely unused while also being large and complex. Therefore, instead of resurrecting _get_events_txn we manually reimplement the bits that are needed directly.

@richvdh
Copy link
Member

richvdh commented Sep 27, 2016

Fix what? is there a bug? what isn't working?

http://chris.beams.io/posts/git-commit/

@richvdh richvdh assigned erikjohnston and unassigned richvdh Sep 27, 2016
@richvdh
Copy link
Member

richvdh commented Sep 27, 2016

sorry, I don't get this.

The storage function `_get_events_txn` was removed everywhere except
from this background reindex. The function was removed due to it being
(almost) completely unused while also being large and complex.
Therefore, instead of resurrecting `_get_events_txn` we manually
reimplement the bits that are needed directly.
@ara4n
Copy link
Member

ara4n commented Sep 27, 2016

the good news is that my synapse on cirrus is no longer spewing a huge stacktrace every few seconds. but yes, @erikjohnston, please can we actually have some context to explain what's going on here?

@erikjohnston erikjohnston assigned richvdh and unassigned erikjohnston Sep 27, 2016
@richvdh
Copy link
Member

richvdh commented Sep 27, 2016

The offending method appears to have been removed in 05e01f2

  • which, given it was in June, makes me wonder why this has only just become a problem?

@richvdh
Copy link
Member

richvdh commented Sep 27, 2016

which, given it was in June, makes me wonder why this has only just become a problem?

Oh, because it's a one-off background update, which will only be a problem for you if you are upgrading directly from before the reindex was introduced, to after 05e01f2.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

And now that I understand all that, this is pretty trivial.

lgtm

@richvdh richvdh assigned erikjohnston and unassigned richvdh Sep 27, 2016
@erikjohnston erikjohnston merged commit 5875a65 into develop Sep 29, 2016
@richvdh richvdh deleted the erikj/fix_reindex branch December 1, 2016 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants