OLD | NEW |
---|---|
1 // Copyright 2013 The Chromium Authors. All rights reserved. | 1 // Copyright 2013 The Chromium Authors. All rights reserved. |
2 // Use of this source code is governed by a BSD-style license that can be | 2 // Use of this source code is governed by a BSD-style license that can be |
3 // found in the LICENSE file. | 3 // found in the LICENSE file. |
4 | 4 |
5 #include "sql/recovery.h" | 5 #include "sql/recovery.h" |
6 | 6 |
7 #include <stddef.h> | 7 #include <stddef.h> |
8 | 8 |
9 #include "base/files/file_path.h" | 9 #include "base/files/file_path.h" |
10 #include "base/format_macros.h" | 10 #include "base/format_macros.h" |
(...skipping 66 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
77 | 77 |
78 // GetMetaVersionNumber() successfully completed. | 78 // GetMetaVersionNumber() successfully completed. |
79 RECOVERY_SUCCESS_META_VERSION, | 79 RECOVERY_SUCCESS_META_VERSION, |
80 | 80 |
81 // Failed in querying recovery meta table. | 81 // Failed in querying recovery meta table. |
82 RECOVERY_FAILED_META_QUERY, | 82 RECOVERY_FAILED_META_QUERY, |
83 | 83 |
84 // No version key in recovery meta table. | 84 // No version key in recovery meta table. |
85 RECOVERY_FAILED_META_NO_VERSION, | 85 RECOVERY_FAILED_META_NO_VERSION, |
86 | 86 |
87 // Automatically recovered entire database successfully. | |
Mark P
2016/04/07 23:09:43
This comment sounds like it could equally apply to
Scott Hess - ex-Googler
2016/04/15 00:38:15
The earlier applies to a specific table, this appl
Mark P
2016/04/19 23:34:27
No, I thought you were being sloppy about database
| |
88 RECOVERY_SUCCESS_AUTORECOVERDB, | |
89 | |
90 // Database was so broken recovery couldn't be entered. | |
91 RECOVERY_FAILED_AUTORECOVERDB_BEGIN, | |
92 | |
93 // Failed to copy schema to autorecover db. | |
94 RECOVERY_FAILED_AUTORECOVERDB_SCHEMA, | |
95 | |
96 // Distinguish failure in querying schema from failure in creating schema. | |
Mark P
2016/04/08 20:12:06
Please clearly state these are subcategories of RE
Scott Hess - ex-Googler
2016/04/15 00:38:14
Done.
| |
97 RECOVERY_FAILED_AUTORECOVERDB_SCHEMACREATE, | |
98 RECOVERY_FAILED_AUTORECOVERDB_SCHEMASELECT, | |
99 | |
100 // Failed querying tables to recover. Should be impossible. | |
101 RECOVERY_FAILED_AUTORECOVERDB_NAMESELECT, | |
102 | |
103 // Failed to recover an individual table. | |
104 RECOVERY_FAILED_AUTORECOVERDB_TABLE, | |
105 | |
106 // Failed to recover [sqlite_sequence] table. | |
107 RECOVERY_FAILED_AUTORECOVERDB_SEQUENCE, | |
108 | |
109 // Failed to recover triggers or views or virtual tables. | |
110 RECOVERY_FAILED_AUTORECOVERDB_AUX, | |
111 | |
87 // Always keep this at the end. | 112 // Always keep this at the end. |
88 RECOVERY_EVENT_MAX, | 113 RECOVERY_EVENT_MAX, |
89 }; | 114 }; |
90 | 115 |
91 void RecordRecoveryEvent(RecoveryEventType recovery_event) { | 116 void RecordRecoveryEvent(RecoveryEventType recovery_event) { |
92 UMA_HISTOGRAM_ENUMERATION("Sqlite.RecoveryEvents", | 117 UMA_HISTOGRAM_ENUMERATION("Sqlite.RecoveryEvents", |
93 recovery_event, RECOVERY_EVENT_MAX); | 118 recovery_event, RECOVERY_EVENT_MAX); |
94 } | 119 } |
95 | 120 |
96 } // namespace | 121 } // namespace |
(...skipping 405 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
502 RecordRecoveryEvent(RECOVERY_FAILED_META_NO_VERSION); | 527 RecordRecoveryEvent(RECOVERY_FAILED_META_NO_VERSION); |
503 } | 528 } |
504 return false; | 529 return false; |
505 } | 530 } |
506 | 531 |
507 RecordRecoveryEvent(RECOVERY_SUCCESS_META_VERSION); | 532 RecordRecoveryEvent(RECOVERY_SUCCESS_META_VERSION); |
508 *version = recovery_version.ColumnInt(0); | 533 *version = recovery_version.ColumnInt(0); |
509 return true; | 534 return true; |
510 } | 535 } |
511 | 536 |
537 namespace { | |
538 | |
539 // Collect each SQL statement from [corrupt.sqlite_master] which starts with | |
540 // |prefix|, and apply it to [main]. Skip 'sqlite_sequence', which will be | |
Mark P
2016/04/07 23:09:43
This comment is too broad. Below you explicitly a
Scott Hess - ex-Googler
2016/04/15 00:38:15
I agree that the example case isn't what I want, b
| |
541 // created on demand by SQLite if any tables use AUTOINCREMENT. | |
Mark P
2016/04/07 23:09:43
This comment should mention the meaning of the ret
Scott Hess - ex-Googler
2016/04/15 00:38:15
Done.
| |
542 bool SchemaCopyHelper(Connection* db, const char* prefix) { | |
543 const size_t prefix_len = strlen(prefix); | |
544 sql::Statement s(db->GetUniqueStatement( | |
545 "SELECT sql FROM corrupt.sqlite_master " | |
546 "WHERE substr(sql, 1, ?)=? AND name<>'sqlite_sequence'")); | |
Mark P
2016/04/07 23:09:43
What happens if prefix_length > length(sql)? What
Scott Hess - ex-Googler
2016/04/15 00:38:14
I think I'll just rewrite this as a full table sca
| |
547 s.BindInt(0, prefix_len); | |
548 s.BindCString(1, prefix); | |
549 | |
550 // TODO(shess): In case of row duplication when balancing a [sqlite_master] | |
551 // btree, this could inject IF NOT EXISTS. I suspect the occurance is too low | |
552 // to be worth the complexity, though. | |
Mark P
2016/04/07 23:09:43
Can you expand this slightly
to be worth complexit
Scott Hess - ex-Googler
2016/04/15 00:38:15
I think I'll remove the comment entirely. A commo
| |
553 | |
Mark P
2016/04/07 23:09:43
This blank line here feels unnecessary (but then I
Scott Hess - ex-Googler
2016/04/15 00:38:15
Done.
| |
554 while (s.Step()) { | |
Mark P
2016/04/07 23:09:43
Should all these executions be wrapped in a transa
Scott Hess - ex-Googler
2016/04/15 00:38:14
Recovery happens to a temporary database, which is
| |
555 std::string sql = s.ColumnString(0); | |
556 sql.insert(prefix_len, "main."); | |
557 if (!db->Execute(sql.c_str())) { | |
558 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_SCHEMACREATE); | |
559 return false; | |
560 } | |
561 } | |
562 if (!s.Succeeded()) { | |
563 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_SCHEMASELECT); | |
564 return false; | |
565 } | |
566 return true; | |
567 } | |
568 | |
569 } // namespace | |
570 | |
571 // This method is derived from SQLite's vacuum.c. VACUUM operates very | |
572 // similarily, creating a new database, populating the schema, then copying the | |
573 // data. | |
574 // | |
575 // TODO(shess): This conservatively uses Rollback() rather than Unrecoverable(). | |
576 // With Rollback(), it is expected that the database will continue to generate | |
577 // errors. Change the failure cases to Unrecoverable() if/when histogram | |
578 // results indicate that everything is working reasonably. | |
579 // | |
580 // static | |
581 void Recovery::RecoverDatabaseOrRaze(Connection* db, | |
582 const base::FilePath& db_path) { | |
583 scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(db, db_path); | |
584 if (!recovery) { | |
585 // TODO(shess): If recovery can't even get started, consider Raze() or | |
586 // Delete(). | |
587 // | |
588 // Reviewing histograms, the majority of Begin() failures are in the ATTACH | |
589 // (788k/month). This may happen for SQLITE_CANTOPEN and SQLITE_NOTADB | |
590 // cases which cannot be processed using writable_schema. The second | |
591 // biggest case is lack of vtable support on iOS, which is fixed but not yet | |
592 // in the channel (175k/month). Lastly, there were 3 failures in a month | |
Mark P
2016/04/08 20:12:06
Please don't put non-normalized numbers in comment
Scott Hess - ex-Googler
2016/04/15 00:38:14
Maybe I'll keep the comment in my local client, si
| |
593 // opening the temporary table - in theory, that may work later, but in | |
594 // practice 3 in 1M might be a reasonable false-positive rate. | |
Mark P
2016/04/08 20:12:06
I think this last sentence isn't worth mentioning
Scott Hess - ex-Googler
2016/04/15 00:38:14
Acknowledged.
| |
595 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_BEGIN); | |
596 db->Poison(); | |
Mark P
2016/04/08 20:12:06
In the header comment, you write the function will
Scott Hess - ex-Googler
2016/04/15 00:38:15
The originating caller's Open()/Close() calls shou
| |
597 return; | |
598 } | |
599 | |
600 #if DCHECK_IS_ON() | |
Mark P
2016/04/08 20:12:06
I don't understand this block of code. That said,
Scott Hess - ex-Googler
2016/04/15 00:38:15
fts3 is a virtual table which we used to use in hi
| |
601 // fts3 has an [x_segdir] table containing a column [end_block INTEGER]. But | |
602 // it actually stores either an integer or a text containing a pair of | |
603 // integers separated by a space. AutoRecoverTable() trust the INTEGER tag | |
604 // when setting up the recover vtable, so those rows get dropped. Setting | |
605 // that column to ANY may work, but maybe it's better to fail hard until | |
606 // someone needs to recover a virtual table. | |
607 if (db->is_open()) { | |
608 sql::Statement s(db->GetUniqueStatement( | |
609 "SELECT 1 FROM sqlite_master WHERE sql LIKE 'CREATE VIRTUAL TABLE %'")); | |
610 DCHECK(!s.Step()) << "Recovery of virtual tables not supported"; | |
611 } | |
612 #endif | |
613 | |
614 // TODO(shess): vacuum.c turns off checks and foreign keys. | |
615 | |
616 // TODO(shess): vacuum.c turns synchronous=OFF for the target. I do not fully | |
617 // understand this, as the temporary db should not have a journal file at all. | |
618 // Perhaps it does in case of cache spill? | |
619 | |
620 // Copy table schema from [corrupt] to [main]. | |
621 if (!SchemaCopyHelper(recovery->db(), "CREATE TABLE ") || | |
622 !SchemaCopyHelper(recovery->db(), "CREATE INDEX ") || | |
623 !SchemaCopyHelper(recovery->db(), "CREATE UNIQUE INDEX ")) { | |
624 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_SCHEMA); | |
625 Recovery::Rollback(std::move(recovery)); | |
Mark P
2016/04/08 20:12:06
Header talks about poisoning; why are you using a
Scott Hess - ex-Googler
2016/04/15 00:38:15
See comment I made about the RecoverDatabaseOrRaze
| |
626 return; | |
627 } | |
628 | |
629 // Run auto-recover against each table, skipping the sequence table. This is | |
630 // necessary because earlier table recovery may populate the sequence table, | |
631 // then copying the sequence table would create duplicates. | |
632 { | |
633 sql::Statement s(recovery->db()->GetUniqueStatement( | |
634 "SELECT name FROM sqlite_master WHERE sql LIKE 'CREATE TABLE %' " | |
635 "AND name!='sqlite_sequence'")); | |
Mark P
2016/04/08 20:12:06
AND != corrupt.sqlite_sequence ?
Scott Hess - ex-Googler
2016/04/15 00:38:15
sqlite_master is implicitly main.sqlite_master .
Mark P
2016/04/19 23:34:27
I'm sorry for making your brain explode. ;-)
| |
636 while (s.Step()) { | |
637 const std::string name = s.ColumnString(0); | |
638 size_t rows_recovered; | |
639 if (!recovery->AutoRecoverTable(name.c_str(), &rows_recovered)) { | |
640 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_TABLE); | |
641 Recovery::Rollback(std::move(recovery)); | |
642 return; | |
643 } | |
644 } | |
645 if (!s.Succeeded()) { | |
646 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_NAMESELECT); | |
647 Recovery::Rollback(std::move(recovery)); | |
648 return; | |
649 } | |
650 } | |
651 | |
652 // Overwrite any sequences created. | |
653 if (recovery->db()->DoesTableExist("corrupt.sqlite_sequence")) { | |
654 ignore_result(recovery->db()->Execute("DELETE FROM main.sqlite_sequence")); | |
655 size_t rows_recovered; | |
656 if (!recovery->AutoRecoverTable("sqlite_sequence", &rows_recovered)) { | |
657 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_SEQUENCE); | |
658 Recovery::Rollback(std::move(recovery)); | |
659 return; | |
660 } | |
661 } | |
662 | |
663 // Copy triggers, views, and virtual tables directly to sqlite_master. Any | |
664 // tables they refer to should already exist. | |
665 // TODO(shess): The rootpage=0 test is from vacuum.c. Consider instead: | |
666 // sql LIKE "CREATE VIRTUAL TABLE %" | |
Mark P
2016/04/08 20:12:06
Do you have a sense of whether this will be differ
Scott Hess - ex-Googler
2016/04/15 00:38:15
I do not fully understand why vacuum.c uses the ro
| |
667 char kCreateMetaItems[] = | |
668 "INSERT INTO main.sqlite_master " | |
669 "SELECT type, name, tbl_name, rootpage, sql " | |
670 "FROM corrupt.sqlite_master " | |
671 "WHERE type='view' OR type='trigger' OR (type='table' AND rootpage=0)"; | |
672 if (!recovery->db()->Execute(kCreateMetaItems)) { | |
673 RecordRecoveryEvent(RECOVERY_FAILED_AUTORECOVERDB_AUX); | |
674 Recovery::Rollback(std::move(recovery)); | |
675 return; | |
676 } | |
677 | |
678 RecordRecoveryEvent(RECOVERY_SUCCESS_AUTORECOVERDB); | |
679 ignore_result(Recovery::Recovered(std::move(recovery))); | |
680 } | |
681 | |
682 // static | |
683 bool Recovery::ShouldRecoverOrRaze(int extended_error) { | |
684 switch (extended_error & 0xFF) { | |
Mark P
2016/04/07 23:09:43
You do the same & 0xFF in connection.cc as well.
Scott Hess - ex-Googler
2016/04/15 00:38:15
Copied language from connection.cc. Unfortunately
| |
685 case SQLITE_CANTOPEN: | |
686 case SQLITE_NOTADB: | |
687 case SQLITE_CORRUPT: | |
688 return true; | |
689 default: | |
690 return false; | |
691 } | |
692 } | |
693 | |
512 } // namespace sql | 694 } // namespace sql |
OLD | NEW |