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

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(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-25 10:19:48
Message-ID: CAG7mmoxk4JPzK4D=4uAYwXzw-U-cj98gQZZvmV12uxcpMNNuZA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Murtuza,

On Wed, Aug 24, 2016 at 3:17 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 2000, in __call__
>>>> >>> return self.wsgi_app(environ, start_response)
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/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/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 1567, in handle_exception
>>>> >>> reraise(exc_type, exc_value, tb)
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 1988, in wsgi_app
>>>> >>> response = self.full_dispatch_request()
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 1641, in full_dispatch_request
>>>> >>> rv = self.handle_user_exception(e)
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 1544, in handle_user_exception
>>>> >>> reraise(exc_type, exc_value, tb)
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>> line 1639, in full_dispatch_request
>>>> >>> rv = self.dispatch_request()
>>>> >>> File "/Users/dpage/.virtualenvs/pgadmin4/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/pgadmin4/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
nodes_connection_status_v4.patch application/octet-stream 88.9 KB
manage_client_connect_lost_v4.patch application/octet-stream 45.7 KB
backend_connection_status_management_v4.patch application/octet-stream 23.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2016-08-25 11:04:37 Re: PATCH: Synonym node for EPAS (pgAdmin4)
Previous Message Murtuza Zabuawala 2016-08-25 09:38:37 Re: PATCH: Synonym node for EPAS (pgAdmin4)