public inbox for [email protected]  
help / color / mirror / Atom feed
From: Dave Page <[email protected]>
To: Nikhil Mohite <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file.
Date: Thu, 3 Jun 2021 10:19:00 +0100
Message-ID: <CA+OCxoxyXvuVgCin3SL7cg8eicGjZ1NKFRkhDGPaP=mgLXXPzg@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0ANKd=_H+P6v7Pp9mLE1uVdrWtSqfn_gWdLowVsyYWTS+Q@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>

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.

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


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: <CA+OCxoxyXvuVgCin3SL7cg8eicGjZ1NKFRkhDGPaP=mgLXXPzg@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