[SERVER-38139] Ban temporary collections in prepared transactions Created: 14/Nov/18  Updated: 29/Oct/23  Resolved: 02/Apr/19

Status: Closed
Project: Core Server
Component/s: Replication
Affects Version/s: None
Fix Version/s: 4.1.10

Type: Task Priority: Major - P3
Reporter: Judah Schvimer Assignee: Dianna Hohensee (Inactive)
Resolution: Fixed Votes: 0
Labels: prepare_errors, txn_storage
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
depends on SERVER-38927 Cache collection 'temp' status on Col... Closed
is depended on by SERVER-37988 recover locks on step up at the begin... Closed
Backwards Compatibility: Fully Compatible
Sprint: Storage NYC 2019-03-25, Storage NYC 2019-04-08
Participants:

 Description   

Temporary collections are dropped on step up regardless of if a transaction is actively prepared on them. This makes it so that they don't really make sense in a transaction. Temporary collections are mainly used internally to mapreduce and agg $out, so no users should be using them in transactions anyways.



 Comments   
Comment by Githook User [ 02/Apr/19 ]

Author:

{'name': 'Dianna Hohensee', 'username': 'DiannaHohensee', 'email': 'dianna.hohensee@10gen.com'}

Message: SERVER-38139 prepareTransaction will error if transaction operations were done against temporary collections
Branch: master
https://github.com/mongodb/mongo/commit/b55d0e6297219efd14fe4d27d0063d61d29a5f43

Comment by Judah Schvimer [ 07/Mar/19 ]

I love the analogy, and the rationale seems reasonable. sgtm.

Comment by Geert Bosch [ 06/Mar/19 ]

I don't think consistency is a sufficient argument. This really is an obscure edge case that users will not ordinarily stumble across. For prepared transactions handling this corner-case matters because of correctness. We'll have to pay a price for that check, but because the distributed transactions are expensive by necessity(in time and resource consumption), the relatively small incremental price of the check will not matter much. Single node transactions are far cheaper, and adding the same checks is relatively expensive, either at run-time or in development time.

While we're OK with a luggage check for an intercontinental flight, requiring that for each subway ride wouldn't fly.

Comment by Judah Schvimer [ 04/Mar/19 ]

For consistency we will also do the above check at commit time for unprepared transactions.

Comment by Geert Bosch [ 04/Mar/19 ]

It has become clear that checking at lock acquisition time is not the right thing to do for a few reasons:

  • The catalog RAII types are at the wrong level, as all lock acquisitions have to go through those, whether part of a transaction or not. Calling into the replication subsystem from the catalog is a layering violation.
  • We do plan to support aggregations with $out side transactions in the future, so disallowing accesses would have to be a temporary stopgap measure.
  • Having a non-versioned cached value on the Collection object goes against our work to make the catalog versioned. Unlike the permanent isCapped attribute, a Collection can go from being temporary to being permanent (typically through rename), or from permanent to temporary (through rollback).

As the real issue is that all collections should be non-temporary at the time of prepare, the proper solution is to check then. As we know at that point that we're in a write transaction, there should be no issue accessing the actual storage catalog.

Comment by Judah Schvimer [ 10/Jan/19 ]

Waiting on SERVER-38927 to implement the Storage Team's preferred solution.

Generated at Thu Feb 08 04:48:04 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.