palantir / atlasdb

Transactional Distributed Database Layer
https://palantir.github.io/atlasdb/
Apache License 2.0
55 stars 14 forks source link

KeyValueService.createTable should create in one transaction #2994

Open mswintermeyer opened 6 years ago

mswintermeyer commented 6 years ago

DbKvs.createTable on postgres creates the table in three transactions, and actually sets the metadata in a fourth. Similar problem for oracle/in memory/etc.

That means that other threads have access to table metadata in a state where the metadata row exists but the metadata isn't yet set. I.e. see https://github.com/palantir/atlasdb/pull/2993. The table creation code should be refactored so everything happens in one transaction.

As a side note, PostgresDdlTable.create already has access to the metadata, and so could set it. I see that it waits till SimpleDbMetadataTable.putMetadata to set (update) it, for code reuse, but that still means it leaves the metadata table in a weird state temporarily.

tboam commented 6 years ago

@mswintermeyer What's the impact of this? Trying to understand the priority for fixing this.

mswintermeyer commented 6 years ago

Just code cleanup. While debugging what could possibly cause the scenario in #2993 where a metadata row existed but didn't have metadata, found the root cause (i.e. create is split into different transactions). But I only bothered to fix the immediate NPE, and just a filed a ticket to track the lower priority of cleaning up the root cause.

tboam commented 6 years ago

Thanks. Giving our current priorities and where we're investing I don't think we'll get to this soon. Moving to the backlog.