cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.04k stars 623 forks source link

Transactions are started in PrepareStatement #1441

Open tli2 opened 6 years ago

tli2 commented 6 years ago

For some unknown reason this bug has been here since a very long time ago (at least a year.)

In traffic_cop.cpp : 306 we prematurely start a transaction as soon as we see that this is a single statement or a begin immediately after parsing, before execute. We attempt to begin the transaction again in the execute phase in some twisted logic.

This is clearly wrong, and makes it extremely hard to reason about what's actually happening in tcop. We haven't been able to seriously exploit this to a bug, and it might just work out in some twisted manner, but we should remove this.

To test it out yourself you can just type ROLLBACK into a psql session and it will keep starting new transactions before immediately rolling them back, as opposed to telling you that no transaction can be rolled back (like postgres does). Alternatively, type in a PREPARE and see a transaction get started.

Just... wat?

tli2 commented 6 years ago

After talking to @ChTimTsubasa it seems like Postgres does this as well so we don't have to invalidate plan on an interleaving like this:

thread1: plan thread1: bind thread2: drop table thread1: execute

The code we have can still be fishy (starting a transaction when parsing an abort, e,g,). More discussion needed. Documentation is definitely needed.