[storage] Migrate QuotaDatabase to scoped sql::Transaction

MERGED2026storage/browser/quotasql
2026. 8. 24.jmsmg 프로필 이미지jmsmg
https://crrev.com/c/8282239

QuotaDatabase의 장수(long-running) 트랜잭션을 deprecated 수동 API(BeginTransactionDeprecated() 등)에서 RAII sql::Transaction 객체로 이행했습니다(crbug.com/40831207). 리뷰 3라운드에 걸쳐 에러 처리 방침이 다듬어졌고, 그 과정에서 업스트림 CL과 같은 줄을 놓고 충돌해 ToT 리베이스로 이어받은 기여입니다.

문제 설명

  • issue url: https://crbug.com/40831207
  • sql::Database의 수동 트랜잭션 메서드 3개는 2024년에 *TransactionDeprecated로 강제 리네임되며 "deprecated and will be removed"로 선언됐지만, 호출처가 남아 있어 삭제하지 못하는 상태였습니다.
  • 이 이슈는 crbug.com/40830473(Raze가 진행 중 트랜잭션을 롤백하지 못하는 문제)을 막고 있습니다. 수동 begin은 대응하는 sql::Transaction 객체가 없어 추적이 불가능하기 때문입니다.
  • QuotaDatabase는 fsync 비용을 줄이기 위해 열 때 BEGIN → 쓰기 배치 → 30초 타이머로 커밋+재BEGIN → 닫을 때 커밋하는 장수 트랜잭션 패턴을 씁니다. 이 사이클 전체가 수동 API 위에 있었습니다.

해결 내용

핵심은 std::optional<sql::Transaction> 멤버입니다. sql::Transaction은 Begin/Commit을 각 1회만 허용하는 일회용·이동 불가 타입이라, 주기 커밋마다 emplace()로 재생성하고 실패 시 reset()하는 관용구가 필요합니다(net/disk_cache/sql, segmentation_platform에 같은 선례가 있습니다).

void QuotaDatabase::Commit() {
  ...
  if (transaction_) {
    transaction_->Commit();
  }
  CHECK(!db_->HasActiveTransactions());
  transaction_.emplace(db_.get());
  if (!transaction_->Begin()) {
    // TODO(crbug.com/40831207): Handle failing to begin the transaction
    // instead of running the following statements outside of one.
    transaction_.reset();
  }
}
  • Commit(): 재-BEGIN 실패 시 크래시하지 않고 트랜잭션 없이(autocommit) 계속 동작하는 자가 치유 형태. 기존에 타이머가 돌고 있으면 멈추지 않던 경로도 함께 정리했습니다.
  • EnsureOpened(): BEGIN 실패를 조용히 무시하던 것을 QuotaError::kDatabaseError 반환 + DB 비활성화로 변경(리뷰 지적 반영). 형제 실패 경로들과 같은 종착 상태로 맞췄습니다.
  • RecoverOrRaze(): sql::Recovery에 DB를 넘기기 전 transaction_.reset()으로 소유를 정리.
  • 헤더에는 수명 주기를 설명하는 주석과 함께 db_ 뒤에 선언해 파괴 순서를 보장했습니다.

리뷰 과정에서 배운 것들

1라운드 후 — "일관성" 함정. 리뷰어(Evan Stade)가 EnsureOpened의 조용한 실패를 지적하자, 같은 논리를 Commit()에도 적용해 DB를 통째로 비활성화하도록 바꿨다가 되돌렸습니다. 두 상황은 대칭이 아니었습니다: EnsureOpened 실패는 "DB를 못 쓴다"이고 에러 반환 통로가 있지만, Commit 실패는 "배치만 깨진다"(데이터는 autocommit으로 정상 저장)이고 반환 통로가 없으며, 호출자가 live sql::Statement를 들고 있을 수 있어 db_.reset()이 API 위반이 됩니다.

2라운드 — I/O 실패는 CHECK 금지. 리베이스로 들어온 업스트림의 CHECK(Begin())을 "업스트림 의도 보존" 명목으로 유지했다가 거부됐습니다. BEGIN은 디스크 I/O라 실패할 수 있고, I/O 실패로 브라우저를 죽여선 안 됩니다. 참고로 그 업스트림 CL은 git cl split의 기계적 치환이라 라인별 의도가 실려 있지 않았습니다("don't over-index on it") — 남의 코드를 근거로 삼기 전에 그 코드가 어떻게 만들어졌는지부터 봐야 한다는 교훈이었습니다.

업스트림 충돌. 리뷰 도중 같은 CHECK들을 다시 쓰는 CL(crrev.com/c/8281077)이 먼저 랜딩해 merge conflict가 났고, ToT 리베이스로 해소했습니다. "CQ가 REBASE_ALWAYS니 리베이스 생략 가능"은 충돌이 없을 때만 성립합니다.

3라운드 — 테스트 헬퍼도 이행 대상. 2차 리뷰어(Steve Becker)가 테스트 헬퍼 IsGlobalTransactionValid()가 여전히 deprecated API로 커밋/재BEGIN하며 상태를 파괴하는 것을 지적해, 읽기 전용 검사(transaction_.has_value() && IsActiveForTesting() && HasActiveTransactions())로 교체했습니다.

테스트 방법

  1. storage_unittests에서 QuotaDatabase 테스트 전원, quota 관련 전체 308개 통과를 각 패치셋마다 확인했습니다(--ozone-platform=headless 필요).
  2. 회귀 테스트 LongRunningTransactionIsReopenedAfterCommit을 추가했습니다. 커밋 후 새 장수 트랜잭션이 열려 있는지 검증하며, 이것이 없으면 Commit()의 재-BEGIN을 통째로 지워도 기존 테스트가 전부 통과합니다.
  3. 변이 테스트로 테스트의 실효성을 증명했습니다: 재-BEGIN 코드를 일부러 제거하니 새 테스트가 두 파라미터 모두 FAILED, 원복하니 통과 — 지운 CHECK_EQ(transaction_nesting)가 지키던 불변식을 새 테스트가 실제로 대체함을 확인했습니다.
  4. 손상/복구 경로(CorruptionRecovery, OpenCorruptedDatabase 등)도 통과했습니다.

배운 점

  • I/O가 실패할 수 있는 연산은 CHECK로 단언하면 안 됩니다. 검사가 싸고 실패가 프로그래밍 오류인 것만 CHECK 대상입니다.
  • base::NotFatalUntil::M148 같은 milestone 인자는 현재 버전 기준으로 이미 만료됐을 수 있습니다. "원본은 조용히 no-op이었다"고 주장하기 전에 원본을 실제로 읽어야 합니다 — 현재 M154에서 그 CHECK들은 전부 fatal이었습니다.
  • 리뷰 원칙을 다른 곳에 확장 적용하기 전에, 두 상황이 정말 같은지부터 확인해야 합니다.
  • 일회용·이동 불가 타입을 멤버로 쓸 때의 std::optional + emplace/reset 관용구.

참고 자료

모든 컨트리뷰션 보기