public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nikhil Mohite <[email protected]>
To: pgadmin-hackers <[email protected]>
Cc: Dave Page <[email protected]>
Subject: Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.
Date: Thu, 3 Jun 2021 15:51:50 +0530
Message-ID: <CAOBg0AMRqY7zRhTWGNHE2EEqWo4fUW5Fnp8OrgBsWgoBwoerhg@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0AMXOs69r69MjQ=0YHn-HgrLAKU+1kva5WXKYx+7pSKg-A@mail.gmail.com>
References: <CAOBg0AOa4EfDwph4Ba2=GV4SSnLJ8vxtqdb=zt8koQNAwuNy9w@mail.gmail.com>
<CA+OCxoz-Qr9ubPZ-1wBrHkF+0B=cC72rEHURyLQfFF4dHSDbhg@mail.gmail.com>
<CAOBg0ANKd=_H+P6v7Pp9mLE1uVdrWtSqfn_gWdLowVsyYWTS+Q@mail.gmail.com>
<CA+OCxoxyXvuVgCin3SL7cg8eicGjZ1NKFRkhDGPaP=mgLXXPzg@mail.gmail.com>
<CAOBg0AMXOs69r69MjQ=0YHn-HgrLAKU+1kva5WXKYx+7pSKg-A@mail.gmail.com>
Hi Team,
PFA updated patch (v2)
On Thu, Jun 3, 2021 at 3:10 PM Nikhil Mohite <[email protected]>
wrote:
> Hi Dave,
>
> On Thu, Jun 3, 2021 at 2:49 PM Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <
>> [email protected]> wrote:
>>
>>> Hi Dave,
>>>
>>> On Thu, Jun 3, 2021 at 1:47 PM Dave Page <[email protected]> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please find the attached patch for RM-6460
>>>>> <https://redmine.postgresql.org/issues/6460;: Need a mechanism to
>>>>> detect a corrupt/broken config DB file.
>>>>>
>>>>> 1. Added checks if all tables added in the model are present in SQLite
>>>>> DB or not.
>>>>> 2. If migrations fail it will backup older file and try migrations
>>>>> with the newly created file.
>>>>> (User will get notification on UI for the location of the backup file
>>>>> and newly created.)
>>>>> 3. If the user deleted any table from SQLite DB pgAdmin will not run
>>>>> on the next restart and it will add the missing table list in the logs.
>>>>>
>>>>
>>>> Surely if any tables have been deleted, it'll fail the check in point 1?
>>>>
>>> Yes, but if the user deletes any table while pgAdmin is running then it
>>> will fail when the user tries to run pgAdmin next time.
>>>
>>
>> Right - but how is the end result of that different from a failed
>> migration that left a table missing? Either way, the end result is the
>> same, and should be handled the same way; i.e. backup/recreate the DB, then
>> warn the user as soon as the UI is up.
>>
> Yes - It should be the same in both conditions, I will send an updated
> patch for this.
>
>>
>
>> I believe the process is simple (and maybe this is what is done and this
>> is just a language issue - I haven't read the patch in detail):
>>
>> - On startup, attempt to create the database/run any migrations as
>> appropriate.
>> - Check that all tables exist, and the schema version is correct.
>> - If there's a problem, backup the file, create a new one from scratch,
>> and warn the user.
>>
>> One thing I did notice when skimming the patch - we have this in the code:
>>
>> + raise RuntimeError(
>> + 'Specified SQLite database file is not valid.')
>>
>> The *user* didn't specify a SQLite database file (nor do they care that
>> it's SQLite at this point). That (and any other similar messages) should
>> probably be rephrased to say something like:
>>
>> "The configuration database file is not valid."
>>
>
>> Thanks!
>>
>>
>>
>>> (If we remove the table from the model it will not check particular
>>> table is present in DB or not. )
>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: https://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EDB: https://www.enterprisedb.com
>>>>
>>>> Regards,
>>> Nikhil Mohite.
>>>
>>
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>> Regards,
> Nikhil Mohite
>
Regards,
Nikhil Mohite
Attachments:
[application/octet-stream] RM_6460_v2.patch (9.6K, 3-RM_6460_v2.patch)
download | inline diff:
diff --git a/web/pgadmin/__init__.py b/web/pgadmin/__init__.py
index 6e395f42..d0c4a25e 100644
--- a/web/pgadmin/__init__.py
+++ b/web/pgadmin/__init__.py
@@ -14,6 +14,8 @@ import os
import sys
import re
import ipaddress
+import traceback
+
from types import MethodType
from collections import defaultdict
from importlib import import_module
@@ -38,8 +40,8 @@ from pgadmin.utils import PgAdminModule, driver, KeyManager
from pgadmin.utils.preferences import Preferences
from pgadmin.utils.session import create_session_interface, pga_unauthorised
from pgadmin.utils.versioned_template_loader import VersionedTemplateLoader
-from datetime import timedelta
-from pgadmin.setup import get_version, set_version
+from datetime import timedelta, datetime
+from pgadmin.setup import get_version, set_version, check_db_tables
from pgadmin.utils.ajax import internal_server_error, make_json_response
from pgadmin.utils.csrf import pgCSRFProtect
from pgadmin import authenticate
@@ -349,6 +351,44 @@ def create_app(app_name=None):
##########################################################################
# Upgrade the schema (if required)
##########################################################################
+ def backup_db_file():
+ """
+ Create a backup of the current database file
+ and create new database file with default settings.
+ """
+ backup_file_name = "{0}.{1}".format(
+ SQLITE_PATH, datetime.now().strftime('%Y%m%d%H%M%S'))
+ os.rename(SQLITE_PATH, backup_file_name)
+ app.logger.error('Exception in database migration.')
+ app.logger.info('Creating new database file.')
+ try:
+ db_upgrade(app)
+ os.environ[
+ 'CORRUPTED_DB_BACKUP_FILE'] = backup_file_name
+ app.logger.info('Database migration completed.')
+ except Exception as e:
+ app.logger.error('Database migration failed')
+ app.logger.error(traceback.format_exc())
+ raise RuntimeError('Migration failed')
+
+ def upgrade_db():
+ """
+ Execute the migrations.
+ """
+ try:
+ db_upgrade(app)
+ os.environ['CORRUPTED_DB_BACKUP_FILE'] = ''
+ except Exception as e:
+ backup_db_file()
+
+ # check all tables are present in the db.
+ is_db_error, invalid_tb_names = check_db_tables()
+ if is_db_error:
+ app.logger.error(
+ 'Table(s) {0} are missing in the'
+ ' database'.format(invalid_tb_names))
+ backup_db_file()
+
with app.app_context():
# Run migration for the first time i.e. create database
from config import SQLITE_PATH
@@ -356,25 +396,34 @@ def create_app(app_name=None):
# If version not available, user must have aborted. Tables are not
# created and so its an empty db
- if not os.path.exists(SQLITE_PATH) or get_version() == -1:
+ version = get_version()
+ if not os.path.exists(SQLITE_PATH) or version == -1:
# If running in cli mode then don't try to upgrade, just raise
# the exception
if not cli_mode:
- db_upgrade(app)
+ upgrade_db()
else:
if not os.path.exists(SQLITE_PATH):
raise FileNotFoundError(
'SQLite database file "' + SQLITE_PATH +
'" does not exists.')
- raise RuntimeError('Specified SQLite database file '
- 'is not valid.')
+ raise RuntimeError(
+ 'The configuration database file is not valid.')
else:
schema_version = get_version()
# Run migration if current schema version is greater than the
# schema version stored in version table
if CURRENT_SCHEMA_VERSION >= schema_version:
- db_upgrade(app)
+ upgrade_db()
+ else:
+ # check all tables are present in the db.
+ is_db_error, invalid_tb_names = check_db_tables()
+ if is_db_error:
+ app.logger.error(
+ 'Table(s) {0} are missing in the'
+ ' database'.format(invalid_tb_names))
+ backup_db_file()
# Update schema version to the latest
if CURRENT_SCHEMA_VERSION > schema_version:
diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py
index edbd491e..5a6a1c46 100644
--- a/web/pgadmin/browser/__init__.py
+++ b/web/pgadmin/browser/__init__.py
@@ -342,6 +342,7 @@ class BrowserModule(PgAdminModule):
list: a list of url endpoints exposed to the client.
"""
return [BROWSER_INDEX, 'browser.nodes',
+ 'browser.check_corrupted_db_file',
'browser.check_master_password',
'browser.set_master_password',
'browser.reset_master_password',
@@ -950,6 +951,19 @@ def form_master_password_response(existing=True, present=False, errmsg=None):
})
[email protected]("/check_corrupted_db_file",
+ endpoint="check_corrupted_db_file", methods=["GET"])
+def check_corrupted_db_file():
+ """
+ Get the corrupted database file path.
+ """
+ file_location = os.environ['CORRUPTED_DB_BACKUP_FILE'] \
+ if 'CORRUPTED_DB_BACKUP_FILE' in os.environ else ''
+ # reset the corrupted db file path in env.
+ os.environ['CORRUPTED_DB_BACKUP_FILE'] = ''
+ return make_json_response(data=file_location)
+
+
@blueprint.route("/master_password", endpoint="check_master_password",
methods=["GET"])
def check_master_password():
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 46dfb4f1..ba84ca41 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -594,7 +594,7 @@ define('pgadmin.browser', [
}, 300000);
obj.set_master_password('');
-
+ obj.check_corrupted_db_file();
obj.Events.on('pgadmin:browser:tree:add', obj.onAddTreeNode, obj);
obj.Events.on('pgadmin:browser:tree:update', obj.onUpdateTreeNode, obj);
obj.Events.on('pgadmin:browser:tree:refresh', obj.onRefreshTreeNode, obj);
@@ -607,7 +607,35 @@ define('pgadmin.browser', [
obj.register_to_activity_listener(document);
obj.start_inactivity_timeout_daemon();
},
+ check_corrupted_db_file: function() {
+ $.ajax({
+ url: url_for('browser.check_corrupted_db_file'),
+ type: 'GET',
+ dataType: 'json',
+ contentType: 'application/json',
+ }).done((res)=> {
+ if(res.data.length > 0) {
+
+ Alertify.alert(
+ 'Warning',
+ 'pgAdmin detected unrecoverable corruption in it\'s SQLite configuration database. ' +
+ 'The database has been backed up and recreated with default settings. '+
+ 'It may be possible to recover data such as query history manually from '+
+ 'the original/corrupt file using a tool such as DB Browser for SQLite if desired.'+
+ '<br><br>Original file: ' + res.data + '<br>Replacement file: ' +
+ res.data.substring(0, res.data.length - 14)
+ )
+ .set({'closable': true,
+ 'onok': function() {
+ },
+ });
+
+ }
+ }).fail(function(xhr, status, error) {
+ Alertify.alert(error);
+ });
+ },
init_master_password: function() {
let self = this;
// Master password dialog
diff --git a/web/pgadmin/setup/__init__.py b/web/pgadmin/setup/__init__.py
index 04571e9e..779a7217 100644
--- a/web/pgadmin/setup/__init__.py
+++ b/web/pgadmin/setup/__init__.py
@@ -11,3 +11,4 @@ from .user_info import user_info
from .db_version import get_version, set_version
from .db_upgrade import db_upgrade
from .data_directory import create_app_data_directory
+from .db_table_check import check_db_tables
diff --git a/web/pgadmin/setup/db_table_check.py b/web/pgadmin/setup/db_table_check.py
new file mode 100644
index 00000000..4da86c27
--- /dev/null
+++ b/web/pgadmin/setup/db_table_check.py
@@ -0,0 +1,32 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2021, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+from pgadmin.model import Version
+from pgadmin.model import db
+
+
+def get_db_table_names():
+ db_table_names = db.metadata.tables.keys() if db.metadata.tables else 0
+ return db_table_names
+
+
+def check_db_tables():
+ is_error = False
+ invalid_tb_names = list()
+ db_table_names = get_db_table_names()
+ # check table is actually present in the db.
+ for table_name in db_table_names:
+ if not db.engine.dialect.has_table(db.engine, table_name):
+ invalid_tb_names.append(table_name)
+ is_error = True
+
+ if is_error:
+ return True, invalid_tb_names
+ else:
+ return False, invalid_tb_names
diff --git a/web/pgadmin/setup/db_version.py b/web/pgadmin/setup/db_version.py
index ad2811a1..50607583 100644
--- a/web/pgadmin/setup/db_version.py
+++ b/web/pgadmin/setup/db_version.py
@@ -16,7 +16,10 @@ def get_version():
except Exception:
return -1
- return version.value
+ if version:
+ return version.value
+ else:
+ return -1
def set_version(new_version):
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.
In-Reply-To: <CAOBg0AMRqY7zRhTWGNHE2EEqWo4fUW5Fnp8OrgBsWgoBwoerhg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox