From: | Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. |
Date: | 2021-06-03 09:40:38 |
Message-ID: | CAOBg0AMXOs69r69MjQ=0YHn-HgrLAKU+1kva5WXKYx+7pSKg-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi Dave,
On Thu, Jun 3, 2021 at 2:49 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
> Hi
>
> On Thu, Jun 3, 2021 at 10:01 AM Nikhil Mohite <
> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> On Thu, Jun 3, 2021 at 1:47 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Thu, Jun 3, 2021 at 7:39 AM Nikhil Mohite <
>>> nikhil(dot)mohite(at)enterprisedb(dot)com> 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
From | Date | Subject | |
---|---|---|---|
Next Message | Nikhil Mohite | 2021-06-03 10:21:50 | Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. |
Previous Message | Dave Page | 2021-06-03 09:19:00 | Re: [pgAdmin][RM-6460]: Need a mechanism to detect a corrupt/broken config DB file. |