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.

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

fix for error-ed out transactions on redshift #1356

Merged
merged 1 commit into from
Mar 20, 2019

Conversation

drewbanin
Copy link
Contributor

Problem

On Redshift (and Postgres?) a single invalid freshness query (ie. for an invalid table) would result in a dead transaction. Subsequent queries used the same connection (and transaction?) and so would fail with with the following error:

Database Error in source my_table (models/schema.yml)
  relation "public.my_table" does not exist

Database Error in source my_table2 (models/schema.yml)
  current transaction is aborted, commands ignored until end of transaction block

Database Error in source my_table3 (models/schema.yml)
  current transaction is aborted, commands ignored until end of transaction block

Solution

Clear the transaction before running freshness queries

Questions

Why are different freshness queries using the same transaction/handle? The fix applied in this PR works well (though it is certainly heavy-handed). Is there a better way to accomplish this?

Changes

  • Reset transactions before running freshness queries
  • add a --threads argument (reasonable)

@drewbanin drewbanin requested a review from beckjake March 20, 2019 18:59
Copy link
Contributor

@beckjake beckjake left a comment

Choose a reason for hiding this comment

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

Why are different freshness queries using the same transaction/handle?

I would guess that the freshness calculation queries do an implicit begin transaction at some point in the adapter sql running process, and never commit or roll back. In wilt chamberlain, this is probably better since we explicitly grab/rename our thread's connection. But maybe not, we should test against it!

The fix applied in this PR works well (though it is certainly heavy-handed). Is there a better way to accomplish this?

We could explicitly commit/rollback after calculating each source freshness instead of before., or do things outside a transaction entirely. I don't think either of those are really any better, and certainly not any easier.

This PR looks good to me!

@drewbanin drewbanin merged commit 3b8d5c0 into dev/stephen-girard Mar 20, 2019
@drewbanin drewbanin deleted the fix/concurrent-transaction-fresness branch March 20, 2019 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants