Re: PATCH/s: RM#1387 - Bad handling of missing connection database server

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH/s: RM#1387 - Bad handling of missing connection database server
Date: 2016-08-29 05:10:43
Message-ID: CAKKotZTdCHaXGbzEQGUmsy3HmGcGURV+wqZ+WaDZXdd1DmOYLg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Ashesh,

Patch is working for me, I did not find any major issues.
But I have fixed some of the issues where alert was not displaying object
name,

PFA add-on patch to your previous patch.

Please review.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Aug 25, 2016 at 3:49 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> Hi Murtuza,
>
>
> On Wed, Aug 24, 2016 at 3:17 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi Ashesh,
>>
>> Here are few scenario I tried which does not like working properly,
>>
>> 1) "properties" dialog do not open for any object while I'm able to open
>> "create" dialog
>>
> Done.
>
>>
>> 2) Click & connect the server, Now go to Statistics tab, you will see
>> blank alert info
>> [Console error] statistics.js:216 Uncaught ReferenceError: that is not
>> defined
>>
> Done.
>
>>
>> 3) Open "Create" dialog fill information, Stop DB & then click on Save
>> button.
>> [Console error] pgadmin.defaults.js:90 Uncaught TypeError:
>> xhr.getResponseHeader is not a function
>>
> Done.
>
>>
>> 4) If pgAdmin4 is running & database is connected, Now if database goes
>> down (eg: stop db service),
>> server stays as connected & there are no error from application when we
>> traverse between tabs.
>>
> Due to point #2.
>
>>
>> 5) If pgAdmin4 is running & one or more server is connected, Now if any
>> database server goes down (eg: stop db service)
>> Refresh entire page now click on expand server-group, loading/expanding
>> server group goes infinite.
>>
> This is not regression of this patch.
> I will share a separate patch for this.
>
> Please try the updated patches.
>
>>
>> Thanks,
>> Murtuza
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, Aug 24, 2016 at 2:17 PM, Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> On Wed, Aug 24, 2016 at 11:25 AM, Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Team,
>>>>
>>>> On Sun, Aug 21, 2016 at 3:55 PM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> On Aug 21, 2016 15:08, "Dave Page" <dpage(at)pgadmin(dot)org> wrote:
>>>>> >
>>>>> > Yeah, but shouldn't it be caught and handled, with a suitable retry
>>>>> yes/no in the UI?
>>>>> And, that realised me, I missed one case, on which I am already
>>>>> working.
>>>>>
>>>> Please find the updated patches.
>>>>
>>> Thanks - Murtuza for point out the issue.
>>>
>>> The patches were not generated from the latest changes.
>>> Please find the updated patches.
>>>
>>> --
>>>
>>> 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>
>>>
>>>>
>>>> --
>>>>
>>>> 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>
>>>>
>>>>> --
>>>>> Thanks & Regards,
>>>>>
>>>>> Ashesh Vashi
>>>>> >
>>>>> >
>>>>> > --
>>>>> > Dave Page
>>>>> > Blog: http://pgsnake.blogspot.com
>>>>> > Twitter: @pgsnake
>>>>> >
>>>>> > EnterpriseDB UK:http://www.enterprisedb.com
>>>>> > The Enterprise PostgreSQL Company
>>>>> >
>>>>> > On 20 Aug 2016, at 12:03, Ashesh Vashi <
>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>> >
>>>>> >> On Fri, Aug 19, 2016 at 9:32 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>> wrote:
>>>>> >>>
>>>>> >>> Hi
>>>>> >>>
>>>>> >>> Can you re-base please? The nodes patch no longer applies.
>>>>> >>>
>>>>> >>> Ignoring that conflict though, the first test I did was to restart
>>>>> the app server when connected to a database. I then (after it restarted) to
>>>>> open the Schemas node in that database; nothing happened at all in the
>>>>> client, but on the server I saw:
>>>>> >>
>>>>> >> This exception suggests that - pgAdmin 4 was not able to establish
>>>>> the connection again during restarting it.
>>>>> >>
>>>>> >> --
>>>>> >>
>>>>> >> Thanks & Regards,
>>>>> >>
>>>>> >> Ashesh Vashi
>>>>> >> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>> >>
>>>>> >>
>>>>> >> http://www.linkedin.com/in/asheshvashi
>>>>> >>>
>>>>> >>>
>>>>> >>> 2016-08-19 16:59:16,321: INFO werkzeug: 127.0.0.1 - - [19/Aug/2016
>>>>> 16:59:16] "GET /browser/schema/nodes/1/1/219759/ HTTP/1.1" 500 -
>>>>> >>> Traceback (most recent call last):
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 2000, in
>>>>> __call__
>>>>> >>> return self.wsgi_app(environ, start_response)
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1991, in
>>>>> wsgi_app
>>>>> >>> response = self.make_response(self.handle_exception(e))
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1567, in
>>>>> handle_exception
>>>>> >>> reraise(exc_type, exc_value, tb)
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1988, in
>>>>> wsgi_app
>>>>> >>> response = self.full_dispatch_request()
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1641, in
>>>>> full_dispatch_request
>>>>> >>> rv = self.handle_user_exception(e)
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1544, in
>>>>> handle_user_exception
>>>>> >>> reraise(exc_type, exc_value, tb)
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1639, in
>>>>> full_dispatch_request
>>>>> >>> rv = self.dispatch_request()
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1625, in
>>>>> dispatch_request
>>>>> >>> return self.view_functions[rule.endpoint](**req.view_args)
>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>> dmin4/lib/python2.7/site-packages/flask/views.py", line 84, in view
>>>>> >>> return self.dispatch_request(*args, **kwargs)
>>>>> >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py",
>>>>> line 235, in dispatch_request
>>>>> >>> return method(*args, **kwargs)
>>>>> >>> File "/Users/dpage/git/pgadmin4/web
>>>>> /pgadmin/browser/server_groups/servers/databases/schemas/__init__.py",
>>>>> line 129, in wrap
>>>>> >>> self.conn = self.manager.connection(did=kwargs['did'])
>>>>> >>> File "/Users/dpage/git/pgadmin4/web
>>>>> /pgadmin/utils/driver/psycopg2/__init__.py", line 1330, in connection
>>>>> >>> raise Exception(msg_active_conn)
>>>>> >>>
>>>>> >>> I realise it could well be caused by the conflict when applying
>>>>> the nodes patch, but just in case it's not... :-)
>>>>> >>>
>>>>> >>> On Fri, Aug 19, 2016 at 12:25 PM, Ashesh Vashi <
>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>> >>>>
>>>>> >>>> Hi Dave/Team,
>>>>> >>>>
>>>>> >>>> I have attached three patches targeting specific area of work.
>>>>> >>>>
>>>>> >>>> -> backend_connection_status_management
>>>>> >>>> * Takes care of the connection status in the psycopg2 driver.
>>>>> And, when the connection is lost, it throws a exception with 503 http
>>>>> status message, and connection lost information in it.
>>>>> >>>> * Contains a change to allow to the flask application to
>>>>> propagate the exceptions even in the release mode.
>>>>> >>>> * Modification for utilising the existing password (while
>>>>> reconnection, if not disconnected explicitly).
>>>>> >>>> * Introduced a new ajax response message 'service_unavailable'
>>>>> (http status code: 503), which suggests temporary service unavailable.
>>>>> >>>>
>>>>> >>>> -> manage_client_connect_lost
>>>>> >>>> * It handles the connection lost for different operations at
>>>>> client (browser) side.
>>>>> >>>> * Using the events to handle this situation.
>>>>> >>>>
>>>>> >>>> -> remove_connection_status_check
>>>>> >>>> * We're currently return precondition_required (HTTP status code:
>>>>> 428) for the same, but - there is no consistent message/information
>>>>> available to identify it reliably as the server connection lost error
>>>>> (client side changes takes care of this situation, but - this patch is
>>>>> recommended for the whole change to work properly). This patch removes the
>>>>> connection status check from the individual nodes.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> TODO:
>>>>> >>>> * Browser tree - node expansion
>>>>> >>>> - When there is an error occurred during expanding the aci-tree
>>>>> node, it does not let the client handle the issue based on the status code,
>>>>> and error message, as it does not expose/share the error details with us.
>>>>> We will have to hack the aci-tree for the same.
>>>>> >>>>
>>>>> >>>> * Query Editor
>>>>> >>>> - Slick grid changes are in progress, hence - I did not touch the
>>>>> code for it, as it will lead to rework for either one of us (me, or
>>>>> Murtuza). I will take care of it as soon as the slick grid changes will be
>>>>> committed.
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>>
>>>>> >>>> Thanks & Regards,
>>>>> >>>>
>>>>> >>>> Ashesh Vashi
>>>>> >>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> http://www.linkedin.com/in/asheshvashi
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> Dave Page
>>>>> >>> Blog: http://pgsnake.blogspot.com
>>>>> >>> Twitter: @pgsnake
>>>>> >>>
>>>>> >>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> >>> The Enterprise PostgreSQL Company
>>>>> >>
>>>>> >>
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>

Attachment Content-Type Size
RM1387_v5_Add-on.patch application/octet-stream 6.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-08-29 06:32:04 pgAdmin 4 commit: Handling the bad/lost connection of a database server
Previous Message Murtuza Zabuawala 2016-08-26 14:57:46 Re: PATCH: SlickGrid integration in query tool (pgAdmin4)