move-coop / parsons

A python library of connectors for the progressive community.
https://www.parsonsproject.org/
Other
261 stars 132 forks source link

Redshift populate_table_from_query silent error #343

Closed elyse-weiss closed 2 years ago

elyse-weiss commented 4 years ago

I am using rs.populate_table_from_query() with an append. We kept not seeing data get appended, and after some investigation in underlying Redshift tables, it looks like it hit this error:

psycopg2.errors.InternalError_: could not complete because of conflict with concurrent transaction

Ideally, the call would error and the error surfaced.

CC: @ydamit @eliotst @jburchard

eliotst commented 4 years ago

@dannyboy15 Would you be able to take a look at this?

dannyboy15 commented 4 years ago

Yeah I can take a look.

eliotst commented 4 years ago

Thanks! Also, take a look at #311, which seems to involve the same function.. if it makes sense to fix while you're already looking at populate_table_from_query.

tiburona commented 4 years ago

@eliotst @dannyboy15 @elyse-weiss What's the status of this? Need any more help? Assuming that there are group Redshift credentials available today I could try to look at it.

(And if so, do you have any advice what behavior would produce this error for checking whether it's caught? I mean, two conflicting concurrent transactions, I guess, but how do you make sure you have two conflicting concurrent transactions? Alternatively, do you just want any error Redshift raises in that method to be raised by Parsons?)

elyse-weiss commented 4 years ago

@tiburona I think we'd love for you to take a look here!

It seems that the function perhaps kicks off the query, but doesn't ensure completion of said query? Perhaps there is some way to have a parameter that is completion_check = True that then hits Redshift system tables? It's just funny because when you use rs.query() you will get an error if you hit a similar Redshift error.

tiburona commented 4 years ago

Yeah that is strange. Okay, I'm happy to look into it (pending getting Redshift credentials). Any pointers on how to reliably produce the bad silent failing behavior, so I know if I've fixed it?

elyse-weiss commented 4 years ago

I think this is where system tables that confirm a query has completed are the best option.

On Wed, Sep 2, 2020 at 12:00 PM Katie Surrence notifications@github.com wrote:

Yeah that is strange. Okay, I'm happy to look into it (pending getting Redshift credentials). Any pointers on how to reliably produce the bad silent failing behavior, so I know if I've fixed it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/move-coop/parsons/issues/343#issuecomment-685832932, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBVWE47XGUL4YMFIV6MQXLSDZT2HANCNFSM4P2OPHIQ .

-- Elyse Ross Weiss Deputy CTO The Movement Cooperative Location: Philadelphia, PA Pronouns: She/Her/Hers

I am currently working part-time as part of TMC's parental leave policy. If this is an urgent matter, please loop in bianca@movementcooperative.org bianca@movementcooperative.org.

*For Technical Support: if you have an urgent need, and it is after 6:00 PM EST, or a weekend, please email *support@movementcooperative.freshdesk.com support@movementcooperative.freshdesk.com

tiburona commented 4 years ago

I think I see what you're saying -- I still think it's important that whoever tries to fix this knows how to reproduce the bad behavior, though. Does that make sense? I'm looking for steps to reproduce an error, or in this case, the lack of an error.

shaunagm commented 4 years ago

@elyse-weiss do you have a bit of sample code or a series of steps that @tiburona could perform, to reproduce the error? It will be hard for her to test whether her fix actually works without that.

dannyboy15 commented 4 years ago

fwiw I was taking a look at this. I was unable to reproduce a 'concurrent transaction' error. However, other errors did behave correctly. i.e. they raised exceptions.

SorenSpicknall commented 2 years ago

Closing for now en lieu of ability to reproduce the error.