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

Add some error wrapping to sync API, use background context for sync #1363

Merged
merged 2 commits into from Aug 28, 2020
Merged

Add some error wrapping to sync API, use background context for sync #1363

merged 2 commits into from Aug 28, 2020

Conversation

neilalexander
Copy link
Contributor

@neilalexander neilalexander commented Aug 28, 2020 •
edited

This is in the vein hope that it might yield something more useful the next time #1159 or #1348 happen, but I can't seem to force the issue to happen.

This also includes a tweak to the context used for the DB transaction until mattn/go-sqlite3#765 is merged.

@neilalexander neilalexander changed the title Add some error wrapping to sync API Add some error wrapping to sync API, use background context for sync Aug 28, 2020
kegsay
kegsay approved these changes Aug 28, 2020
View reviewed changes
Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

This closes #1348 and #1159 - we knew it was double txn and related to contexts, so it feels like the upstream sqlite3 issue really is the cause here. We can re-open if we encounter it again (otherwise when will we ever close them, how long do we wait?!)

This was linked to issues Aug 28, 2020
Possible transaction in transaction error #1348
Closed
sqlite/http: /sync 500s forever #1159
Closed
@neilalexander neilalexander merged commit c42c705 into master Aug 28, 2020
@neilalexander neilalexander deleted the neilalexander/leakingtxn branch August 28, 2020 12:28
anandv96 pushed a commit to anandv96/dendrite that referenced this pull request Sep 1, 2020
@neilalexander
Add some error wrapping to sync API, use background context for sync (m…
ac063a8 
…atrix-org#1363)

* Add some error wrapping to sync API

* Don't use request context for BeginTx until mattn/go-sqlite3#764 is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kegsay kegsay kegsay approved these changes

Assignees
No one assigned
Labels
C-Sync-API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possible transaction in transaction error sqlite/http: /sync 500s forever
2 participants
@neilalexander @kegsay