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

eliminate race condition when updating types #26

Closed
chadwhitacre opened this issue Oct 12, 2013 · 7 comments · Fixed by #43
Closed

eliminate race condition when updating types #26

chadwhitacre opened this issue Oct 12, 2013 · 7 comments · Fixed by #43

Comments

@chadwhitacre
Copy link
Collaborator

We ran into an issue over on gratipay/gratipay.com#1583 in that psycopg2 caches information about composite types, so if you alter the type in the database then the cache becomes stale, and the next time you request a record of the composite type, you get a DataError. From @dvarrazzo:

The composite cache is local to where register_composite was called (globally, the connection, or the cursor). You can avoid the problem using it in a restricted scope (e.g. on a cursor, and dropping the cursor after the type is changed). Alternatively just rerun register_composite with the same arguments after changing the type.

And over on http://psycopg.lighthouseapp.com/projects/62710/tickets/183:

Caching of the attribute is necessary but it's local to the scope you have registered the composite.
Avoid using register_composite with global=True: use it on the connection or on the cursor, and dispose the connection or the cursor after the data type is changed. Alternatively just call register_composite again.

How do we eliminate this race condition?

@chadwhitacre
Copy link
Collaborator Author

I've proposed on http://psycopg.lighthouseapp.com/projects/62710-psycopg/tickets/183 that, as the race condition appears ineliminable, psycopg2 might raise a subclass of DataError instead of DataError itself (which is defined by DB-API 2.0 and is used for several error conditions in psycopg2; the spec seems to allow for subclassing). That way we could retry the query after re-registering composites, and make this transparent to consumers.

@chadwhitacre
Copy link
Collaborator Author

Follow-up over there:

Hello Chad,

the use case for such subclass is fairly restricted, and I think it would be more the hassle to document it than its general usefulness, not to mention that people is usually unhappy to get a new feature in rel. 2.5.2.
So my suggestion is you make your own subclass of the adapter, which should be something as easy as (untested):

class CompositeChanged(DataError):
    pass

class OurCompositeCaster(CompositeCaster):
    def parse(self, s, curs):
        try:
            return super(OurCompositeCaster, self).parse(s, curs)
        except DataError, e:
            raise CompositeChanged(str(e))
this should do everything you need.

Sounds sensible, assuming we can guarantee that there are no other DataErrors that parse could raise.

@dvarrazzo
Copy link

The CompositeCaster calls recursively other typecasters, so the possibility another DataError is raised cannot be ruled out. If you want to be safer you can grab the entire CompositeCaster class code from the extras module and customize it at leisure: it is pretty much self-contained. In either cases use register_composite(..., factory=OurCompositeCaster) to use it.

@chadwhitacre
Copy link
Collaborator Author

We could also inspect exc.args[0], of course.

@chadwhitacre
Copy link
Collaborator Author

Or we could re-perform the original test:

        tokens = self.tokenize(s)
        if len(tokens) != len(self.atttypes):
            raise psycopg2.DataError(
                "expecting %d components for the type %s, %d found instead" %
                (len(self.atttypes), self.name, len(tokens)))

https://github.com/psycopg/psycopg2/blob/497247a52836e971b0b5a5779d0c5c60b98e654d/lib/extras.py#L835-838

@chadwhitacre
Copy link
Collaborator Author

Well, at least we're not the only ones with this problem. :-)

The question is: how do we coordinate continuously pushing to production with schema changes? That is a little challenging. We develop behind feature flags. So, in theory, we should be able to build these features, for the most part, as much as we want, and once the schema is in place, turn the flag on, and everything's okay. We have one day a week that's our schema change day.

I feel like schema changes are the one thing ... they're the outlier in the nice happy continuous deployment. Continuous deployment is so easy and so alluring—to get the code out there—but yeah, there's no real great way to automate schema changes, at least not that we've stumbled on yet. Maybe someone will solve that.

https://www.youtube.com/watch?v=eenrfm50mXw#t=28m59s

@chadwhitacre
Copy link
Collaborator Author

In particular, what's the state of schema changes during continuous deployment? Any progress?

https://twitter.com/whit537/status/583815888354336768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants