On Mon, Jan 8, 2018 at 7:26 PM, Dave Page <[email protected]> wrote: > > > On Mon, Jan 8, 2018 at 1:18 PM, Harshal Dhumal < > [email protected]> wrote: > >> >> On Mon, Jan 8, 2018 at 6:11 PM, Dave Page <[email protected]> wrote: >> >>> >>> >>> On Mon, Jan 8, 2018 at 12:37 PM, Harshal Dhumal < >>> [email protected]> wrote: >>> >>>> On Mon, Jan 8, 2018 at 5:15 PM, Dave Page <[email protected]> wrote: >>>> >>>>> HI >>>>> >>>>> On Mon, Jan 8, 2018 at 11:41 AM, Harshal Dhumal < >>>>> [email protected]> wrote: >>>>> >>>>>> On Mon, Jan 8, 2018 at 4:34 PM, Dave Page <[email protected]> wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> On Fri, Jan 5, 2018 at 7:50 AM, Harshal Dhumal < >>>>>>> [email protected]> 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 >
