Re: RM2815: Relogin to pgAdmin from sqleditor/datadrid if session exprires

Can you rebase this please?

On Wed, Jan 10, 2018 at 9:47 AM, Harshal Dhumal <
harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Please find rebased patch.
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Tue, Jan 9, 2018 at 9:21 PM, Harshal Dhumal <
> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> Please find updated patch where we are notifying user about connection
>> reset.
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Mon, Jan 8, 2018 at 7:39 PM, Harshal Dhumal <
>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>
>>> On Mon, Jan 8, 2018 at 7:26 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Jan 8, 2018 at 1:18 PM, Harshal Dhumal <
>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>> On Mon, Jan 8, 2018 at 6:11 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Jan 8, 2018 at 12:37 PM, Harshal Dhumal <
>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> On Mon, Jan 8, 2018 at 5:15 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> HI
>>>>>>>>
>>>>>>>> On Mon, Jan 8, 2018 at 11:41 AM, Harshal Dhumal <
>>>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> On Mon, Jan 8, 2018 at 4:34 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> On Fri, Jan 5, 2018 at 7:50 AM, Harshal Dhumal <
>>>>>>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Further details:
>>>>>>>>>>>
>>>>>>>>>>> 1. If session is expired and user performs any action from
>>>>>>>>>>> sqleditor that makes ajax call
>>>>>>>>>>> then in ajax error call back user can check and handle login
>>>>>>>>>>> related error using code snippet.
>>>>>>>>>>>
>>>>>>>>>>> if (pgAdmin.Browser.UserManagement.is_pga_login_required(xhr)) {
>>>>>>>>>>> return pgAdmin.Browser.UserManagement.pga_login();
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> Where is xhr is standard xhr or jqxhr object.
>>>>>>>>>>>
>>>>>>>>>>> 2. Similarly for connection lost (only maintenance db connection
>>>>>>>>>>> as we can recover or reconnect other
>>>>>>>>>>> connections if maintenance db connection is alive). It will
>>>>>>>>>>> attempt to create/reconnect connection without
>>>>>>>>>>> asking password (to handle passwordless connection, or saveed
>>>>>>>>>>> password or password from pgpass file)
>>>>>>>>>>> If connection to database still fails then it'll prompt for
>>>>>>>>>>> password.
>>>>>>>>>>>
>>>>>>>>>>> Code snippet:
>>>>>>>>>>> SqlEditorController.handle_connection_lost();
>>>>>>>>>>> once connection lost is detected one can call
>>>>>>>>>>> handle_connection_lost() to reconnect.
>>>>>>>>>>>
>>>>>>>>>>> 3. We maintain some additional data in session to maintain
>>>>>>>>>>> affinity between
>>>>>>>>>>> each sqleditor/datagrid instance to backend database connection.
>>>>>>>>>>> However if session expires and user
>>>>>>>>>>> re-loggins then we need to first restore affinity between
>>>>>>>>>>> sqleditor to backend database before we can start
>>>>>>>>>>> using query tool.
>>>>>>>>>>>
>>>>>>>>>>> Code snippet:
>>>>>>>>>>>
>>>>>>>>>>> if(is_new_transaction_required(xhr)) {
>>>>>>>>>>> SqlEditorController.init_transaction();
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> (note: I haven't looked at the code yet)
>>>>>>>>>>
>>>>>>>>>> How does this handle re-establishment of the connection
>>>>>>>>>> mid-transaction, or, if the user has modified any session variables?
>>>>>>>>>>
>>>>>>>>>> ServeManager and Connection Manager are written in a such way
>>>>>>>>> that if any connection is lost except maintenance db connection
>>>>>>>>> then we can re-connect or create new connection without prompting
>>>>>>>>> for database password and if maintenance db connection is lost
>>>>>>>>> then It prompts for password.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Right.
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regarding session variables as long as flask session is not
>>>>>>>>> expired we uses same session variables. But in case of user logout (due to
>>>>>>>>> flask session expire) we create new transaction id and sets new
>>>>>>>>> session variables for that particular Sql editor /datagrid instance.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I mean DB session variables (and related things). For example, if
>>>>>>>> the user executed queries such as the following, then they absolutely need
>>>>>>>> to know if the session got reset:
>>>>>>>>
>>>>>>>
>>>>>>> Ok.
>>>>>>> Then in this case can we notify user about the same. That we're
>>>>>>> unable to restore old database connection and created new one and therefore
>>>>>>> any DB session variables were set/modified (like SET
>>>>>>> CLIENT_ENCODING..., SET DATESTYLE...) are lost (or similar message).
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> CREATE TEMPORARY TABLE ....
>>>>>>>> SET ROLE ...
>>>>>>>> SET [various other options]
>>>>>>>>
>>>>>>>> If the user has done any of those things (or similar things that I
>>>>>>>> haven't thought of), then we cannot just blindly reset the database
>>>>>>>> connection.
>>>>>>>>
>>>>>>>
>>>>>>> We only create new connection if flask session was expired as we
>>>>>>> lost transaction id associated with Sql editor/datagrid and therefore
>>>>>>> unique connection id
>>>>>>> given to connection which was associated to Sql editor/datagrid. In
>>>>>>> this case we can notify about same (as stated above).
>>>>>>>
>>>>>>
>>>>>> If it's only the flask session we're resetting, not the database
>>>>>> connection, we won't need to warn the user will we?
>>>>>>
>>>>>
>>>>> The problem here is that if flask session is reset then we lose the
>>>>> information about which connection was associate with which query
>>>>> tool/datagrid.
>>>>>
>>>>> lets say If user have opened 3 query tools with same database
>>>>> therefore we'll have 3 separate connections
>>>>> (each will have unique connection id). Now this information that which
>>>>> connection is associate with which query
>>>>> tool is lost and also unique connection id. So there is no way that we
>>>>> can get that connection
>>>>> from connection manager ( *manager.connection(did=<some did>,
>>>>> conn_id=<unique connection id>)* ).
>>>>>
>>>>> So even database connection was not lost and only flask session was
>>>>> reset we need to create new connection. So I think we'll need to warn user.
>>>>>
>>>>
>>>> Agreed... and ensure the database connection is fully reset.
>>>>
>>>>
>>>>>
>>>>> Also If we save connection id to client side (in browser in JS) still
>>>>> we won't be able to get same connection
>>>>> even though we know connection id in case of flask session reset. As
>>>>> for each logged in user (pgAdmin user)
>>>>> but for same database server we create separate ServerManager (and
>>>>> therefore separate connection pool)
>>>>> and flask session reset is same as if same user is logged in from
>>>>> another browser.
>>>>>
>>>>>
>>>>>>
>>>>>> But... what if the database connection has also been lost in the
>>>>>> meantime. Would we handle that?
>>>>>>
>>>>> We create new connection irrespective of old connection state if flask
>>>>> session was reset as explained above.
>>>>>
>>>>
>>>> OK.
>>>>
>>>> So... we need to always warn the user that the connection has been
>>>> reset, so they know if they've lost previous GUC changes or temp tables
>>>> etc, and conversely, we need to reset the database connection to ensure
>>>> thatGUC changes or temp tables don't end up getting re-associated with the
>>>> wrong session. Sound right?
>>>>
>>>> Yes
>>>
>>>
>>>> I assume updates to the patch are required?
>>>>
>>> Yes. I'll send updated one.
>>>
>>>
>>>>
>>>>
>>>> --
>>>> 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