Re: PATCH: RM# 1679 - Background process for "restore" not reporting status back to pgAdmin

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: RM# 1679 - Background process for "restore" not reporting status back to pgAdmin
Date: 2017-01-11 06:58:36
Message-ID: CAG7mmozSqAa4edH2UfQPLNVL7oAye11J-L6MDyZUGvxPnx3nWQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

On Wed, Jan 11, 2017 at 12:03 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> Attempting a backup with an incorrect bin path worked as expected with
> this version, but once I corrected the path and started another backup, I
> got the following:
>
> 2017-01-11 11:45:22,627: INFO werkzeug: 127.0.0.1 - - [11/Jan/2017
> 11:45:22] "GET /misc/bgprocess/?_=1484115208342 HTTP/1.1" 500 -
> Traceback (most recent call last):
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1994, in __call__
> return self.wsgi_app(environ, start_response)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1985, in wsgi_app
> response = self.handle_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1540, in handle_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1982, in wsgi_app
> response = self.full_dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1614, in full_dispatch_request
> rv = self.handle_user_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1517, in handle_user_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1612, in full_dispatch_request
> rv = self.dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1598, in dispatch_request
> return self.view_functions[rule.endpoint](**req.view_args)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask_login.py",
> line 792, in decorated_view
> return func(*args, **kwargs)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/__init__.py",
> line 70, in index
> return make_response(response=BatchProcess.list())
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/processes.py",
> line 410, in list
> status, updated = BatchProcess.update_process_info(p)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/processes.py",
> line 393, in update_process_info
> except json.JSONDecodeError as e:
> AttributeError: 'module' object has no attribute 'JSONDecodeError'
>
> I fixed that by adding:
>
> try:
> JSONDecodeError = json.JSONDecodeError
> except AttributeError:
> JSONDecodeError = ValueError
>
> at line 380 of processes.py, then updating the exception handler
> accordingly.
>
> I then tried to backup an entire server, and got:
>
> 2017-01-11 11:57:21,841: INFO werkzeug: 127.0.0.1 - - [11/Jan/2017
> 11:57:21] "GET /misc/bgprocess/?_=1484115967417 HTTP/1.1" 500 -
> Traceback (most recent call last):
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1994, in __call__
> return self.wsgi_app(environ, start_response)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1985, in wsgi_app
> response = self.handle_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1540, in handle_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1982, in wsgi_app
> response = self.full_dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1614, in full_dispatch_request
> rv = self.handle_user_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1517, in handle_user_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1612, in full_dispatch_request
> rv = self.dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1598, in dispatch_request
> return self.view_functions[rule.endpoint](**req.view_args)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask_login.py",
> line 792, in decorated_view
> return func(*args, **kwargs)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/__init__.py",
> line 70, in index
> return make_response(response=BatchProcess.list())
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/processes.py",
> line 415, in list
> status, updated = BatchProcess.update_process_info(p)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/misc/bgprocess/processes.py",
> line 390, in update_process_info
> if data['end_time']:
> KeyError: u'end_time'
>
> The status file contains:
>
> {"start_time": "2017-01-11 06:27:20.939703 +0000", "pid": 49363,
> "exit_code": 0, "end_time": "2017-01-11 06:27:28.613456 +0000"}
>

Thanks for reviewing the patch.
Please find the updated patch.

Changes:
- Using now ValueError instead of json.JSONDecodeError, which is subclass
of ValueError, to allow to it to work with 2.6+ python.
- Checking the existence of value in dict before accessing it.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com/>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

>
> On Tue, Jan 10, 2017 at 5:47 PM, Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Team,
>>
>> In new implementation - we are forking the process-executor on the POSIX
>> system to run the process in true daemon mode.
>> And, we were updating the status in the configuration file (which is
>> sqlite database), it was crashing while opening the configuration file, and
>> couldn't update the status.
>> Reference: https://bugs.python.org/issue27126
>>
>> In order to resolve the issue, we're now creating a 'status' file for the
>> process information.
>> And, it will be used to check the status information about the process.
>>
>> Thanks Dave for helping find out the root cause of the issue, and point
>> out the correct reference point.
>> Please find the updated patch with the latest changes.
>>
>> I have also attached a patch to make the restore, and backup routes more
>> REST API compatible.
>>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
>> On Mon, Dec 19, 2016 at 5:28 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> On Mon, Dec 19, 2016 at 11:52 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>> >
>>> >
>>> > On Fri, Dec 16, 2016 at 10:16 AM, Ashesh Vashi
>>> > <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>> >>
>>> >> Hi Dave,
>>> >>
>>> >> On Mon, Dec 12, 2016 at 4:01 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>> >>>
>>> >>> Hi,
>>> >>>
>>> >>> On Fri, Dec 9, 2016 at 9:16 AM, Ashesh Vashi
>>> >>> <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>> >>>>
>>> >>>> Hi Dave,
>>> >>>>
>>> >>>> Please find the patch to resolve the issue reported in RM #1679
>>> >>>>
>>> >>>> This will take care of:
>>> >>>> - Find the appropriate available Python interpreter to execute the
>>> >>>> process_executor.py.
>>> >>>> In case of WSGI or Runtime, it was not properly find the
>>> interpreter
>>> >>>> required to execute that script. Also, on windows - we should give
>>> priority
>>> >>>> to the windowless python interpreter (if available).
>>> >>>> - Execute the process_executor.py script with proper platform
>>> dependent
>>> >>>> flags to run it as daemon.
>>> >>>> - Run the process_executor.py in proper daemon mode. It helps to
>>> run the
>>> >>>> long running processes like backup, restore, etc.
>>> >>>> On windows, run the process_executor.py from process_executor.py
>>> in
>>> >>>> detached mode to allow the child process to run in detached mode.
>>> >>>> On POSIX, fork the process_executor.py to allow the child process
>>> to
>>> >>>> run in daemon mode.
>>> >>>> Also - listen the signal like SIGINT, SIGTERM, so that - the child
>>> >>>> does not kill, or hangup (It used to happen.
>>> >>>>
>>> >>>>
>>> >>>> NOTE:
>>> >>>> This patch does not take care of the unicode errors in the path. I
>>> will
>>> >>>> send a separate patch for the same.
>>> >>>
>>> >>>
>>> >>> Unfortunately my first test of this failed:
>>> >>>
>>> >>> SERVER_MODE = False
>>> >>> Python 2.7.11 (Mac default)
>>> >>> Running in Chrome
>>> >>>
>>> >>> I ran a backup of a database, and got the green backup initiated
>>> popup...
>>> >>> then, nothing. Upon checking my config, I found I had the PostgreSQL
>>> Bin
>>> >>> Path set to "$DIR/a/b/c", which clearly won't work. So, we're not yet
>>> >>> detecting failure to start a process properly.
>>> >>
>>> >> During exception handling, the logger's encoding was not set properly
>>> >> during initialisation, that was resulting into an error.
>>> >>
>>> >> Also - sometime the process execution does not start quickly enough to
>>> >> list down the process execution properly.
>>> >> Hence - we should check the process list after some time.
>>> >>
>>> >> Please find the update patch with the above both problems resolved.
>>> >
>>> >
>>> > Same results with the new patch - either with a correct bin path or an
>>> > incorrect one, I see the green notification that the backup has
>>> started,
>>> > then nothing.
>>> >
>>> > Sidenote: I don't even see anything at all in the console output. I
>>> think we
>>> > should at least spit out a debug message showing the command line we're
>>> > executing the launcher with, and ideally include any other details we
>>> can
>>> > such as job IDs etc.
>>>
>>> BTW; attached is my process table from the SQLite database.
>>>
>>> I believe the last two rows are from my test today. The out and err
>>> logs for both are empty files.
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
RM1679_v7.patch application/octet-stream 35.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-01-11 13:19:43 Re: PATCH: RM# 1679 - Background process for "restore" not reporting status back to pgAdmin
Previous Message Dave Page 2017-01-11 06:33:03 Re: PATCH: RM# 1679 - Background process for "restore" not reporting status back to pgAdmin