public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashesh Vashi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: RM#1387 [Add-on PATCH] Bad handling of missing connection database server
Date: Tue, 30 Aug 2016 21:35:22 +0530
Message-ID: <CAG7mmoxUL-H1poTGZz4KyMd9Tw3Po4yRDw_GK1Wu6w4YTK1WXA@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmoz6hEi1BE=nSs2fGt_y5gpkpj6gDEp+WMAWJrJwepVHYw@mail.gmail.com>
References: <CAG7mmoz6hEi1BE=nSs2fGt_y5gpkpj6gDEp+WMAWJrJwepVHYw@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
On Tue, Aug 30, 2016 at 7:16 PM, Ashesh Vashi <[email protected]
> wrote:
> Hi Dave,
>
> Please find the add-on patch on top of the current change.
>
> Can you please take a look at it?
> This mainly works on the postgres driver to make an attempt to reconnect
> the server.
>
One more attempt with some more corner cases handling.
* Handled the connection-lost, and object gone error on client side during
'refresh' operation.
* Handle the reconnection more consistently (even during cursor object
creation).
Please take a look at 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;
>
> --
>
> 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;
>
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] RM1387_addon_v2.patch (23.6K, 3-RM1387_addon_v2.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index 4271199..0b6c6ca 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -818,7 +818,7 @@ class ServerNode(PGChildNodeView):
'servers/password.html',
server_label=server.name,
username=server.username,
- errmsg=e.message if e.message else str(e),
+ errmsg=getattr(e, 'message', str(e)),
_=gettext
)
)
diff --git a/web/pgadmin/browser/templates/browser/js/browser.js b/web/pgadmin/browser/templates/browser/js/browser.js
index 0dbce17..965bbae 100644
--- a/web/pgadmin/browser/templates/browser/js/browser.js
+++ b/web/pgadmin/browser/templates/browser/js/browser.js
@@ -8,7 +8,7 @@ define('pgadmin.browser',
'pgadmin.browser.node', 'pgadmin.browser.collection'
],
-function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
+function(require, $, _, S, Bootstrap, pgAdmin, Alertify, CodeMirror) {
// Some scripts do export their object in the window only.
// Generally the one, which do no have AMD support.
@@ -40,6 +40,7 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
_.each(data, function(d){
d._label = d.label;
d.label = _.escape(d.label);
+ data._inode = data.inode;
})
return data;
};
@@ -991,6 +992,7 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
}
_data._label = _data.label;
_data.label = _.escape(_data.label);
+ _data._inode = _data.inode;
traversePath();
},
@@ -1321,6 +1323,7 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
ctx.pI.push(_old);
_new._label = _new.label;
_new.label = _.escape(_new.label);
+ _new._inode = _new.inode;
if (_old._pid != _new._pid) {
ctx.op = 'RECREATE';
@@ -1351,7 +1354,7 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
ctx.i = null;
ctx.d = null;
} else {
- isOpen = this.tree.isInode(_i) && this.tree.isOpen(_i);
+ isOpen = (this.tree.isInode(_i) && this.tree.isOpen(_i));
}
ctx.branch = ctx.t.serialize(
@@ -1383,7 +1386,8 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
return;
}
var fetchNodeInfo = function(_i, _d, _n) {
- var url = _n.generate_url(_i, 'nodes', _d, true);
+ var info = _n.getTreeNodeHierarchy(_i),
+ url = _n.generate_url(_i, 'nodes', _d, true);
$.ajax({
url: url,
@@ -1396,6 +1400,7 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
data._label = data.label;
data.label = _.escape(data.label);
+ data._inode = data.inode;
var d = ctx.t.itemData(ctx.i);
_.extend(d, data);
ctx.t.setLabel(ctx.i, {label: _d.label});
@@ -1417,21 +1422,44 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
}
},
error: function(jqx, error, status) {
- var p = ctx.t.parent(ctx.i);
-
- if (!p)
- return;
-
- ctx.t.remove(ctx.i, {
- success: function() {
- // Try to refresh the parent on error
- try {
- pgBrowser.Events.trigger(
- 'pgadmin:browser:tree:refresh', p
- );
- } catch(e) {}
+ if (
+ !Alertify.pgHandleItemError(
+ xhr, error, message, {item: _i, info: info}
+ )
+ ) {
+ var msg = xhr.responseText,
+ contentType = xhr.getResponseHeader('Content-Type'),
+ msg = xhr.responseText,
+ jsonResp = (
+ contentType &&
+ contentType.indexOf('application/json') == 0 &&
+ $.parseJSON(xhr.responseText)
+ ) || {};
+
+ if (xhr.status == 410 && jsonResp.success == 0) {
+ var p = ctx.t.parent(ctx.i);
+
+ ctx.t.remove(ctx.i, {
+ success: function() {
+ if (p) {
+ // Try to refresh the parent on error
+ try {
+ pgBrowser.Events.trigger(
+ 'pgadmin:browser:tree:refresh', p
+ );
+ } catch(e) {}
+ }
+ }
+ });
}
- });
+
+ Alertify.pgNotifier(
+ error, xhr, "{{ _("Error retrieving details for the node.") }}",
+ function() {
+ console.log(arguments);
+ }
+ );
+ }
}
});
}.bind(this);
@@ -1466,6 +1494,13 @@ function(require, $, _, S, Bootstrap, pgAdmin, alertify, CodeMirror) {
console.log(arguments);
}
});
+ } else if (!this.tree.isInode(_i) && d._inode) {
+ this.tree.setInode(_i, {
+ success: fetchNodeInfo.bind(this, _i, d, n),
+ fail: function() {
+ console.log(arguments);
+ }
+ });
} else {
fetchNodeInfo(_i, d, n);
}
diff --git a/web/pgadmin/utils/driver/psycopg2/__init__.py b/web/pgadmin/utils/driver/psycopg2/__init__.py
index e3a9e96..d745fee 100644
--- a/web/pgadmin/utils/driver/psycopg2/__init__.py
+++ b/web/pgadmin/utils/driver/psycopg2/__init__.py
@@ -131,6 +131,9 @@ class Connection(BaseConnection):
* _release()
- Release the connection object of psycopg2
+ * _reconnect()
+ - Attempt to reconnect to the database
+
* _wait(conn)
- This method is used to wait for asynchronous connection. This is a
blocking call.
@@ -181,6 +184,8 @@ class Connection(BaseConnection):
self.row_count = 0
self.__notices = None
self.password = None
+ self.wasConnected = False
+ self.reconnecting = False
super(Connection, self).__init__()
@@ -233,7 +238,8 @@ class Connection(BaseConnection):
encpass = self.password or getattr(mgr, 'password', None)
# Reset the existing connection password
- self.password = None
+ if self.reconnecting is not False:
+ self.password = None
if encpass:
# Fetch Logged in User Details.
@@ -301,8 +307,44 @@ Failed to connect to the database server(#{server_id}) for connection ({conn_id}
return False, msg
self.conn = pg_conn
- self.__backend_pid = pg_conn.get_backend_pid()
+ self.wasConnected = True
+ try:
+ status, msg = self._initialize(conn_id, **kwargs)
+ except Exception as e:
+ current_app.logger.exception(e)
+ self.conn = None
+ if not self.reconnecting:
+ self.wasConnected = False
+ raise e
+
+ if status:
+ mgr._update_password(encpass)
+ else:
+ if not self.reconnecting:
+ self.wasConnected = False
+
+ return status, msg
+
+ def _initialize(self, conn_id, **kwargs):
self.execution_aborted = False
+ self.__backend_pid = self.conn.get_backend_pid()
+
+ setattr(g, "{0}#{1}".format(
+ self.manager.sid,
+ self.conn_id.encode('utf-8')
+ ), None)
+
+ status, cur = self.__cursor()
+ formatted_exception_msg = self._formatted_exception_msg
+ mgr = self.manager
+
+ def _execute(cur, query, params=None):
+ try:
+ self.__internal_blocking_execute(cur, query, params)
+ except psycopg2.Error as pe:
+ cur.close()
+ return formatted_exception_msg(pe, False)
+ return None
# autocommit flag does not work with asynchronous connections.
# By default asynchronous connection runs in autocommit mode.
@@ -313,22 +355,22 @@ Failed to connect to the database server(#{server_id}) for connection ({conn_id}
self.conn.autocommit = True
register_date_typecasters(self.conn)
- status, res = self.execute_scalar("""
+ status = _execute(cur, """
SET DateStyle=ISO;
SET client_min_messages=notice;
SET bytea_output=escape;
SET client_encoding='UNICODE';""")
- if not status:
+ if status is not None:
self.conn.close()
self.conn = None
- return False, res
+ return False, status
if mgr.role:
- status, res = self.execute_scalar(u"SET ROLE TO %s", [mgr.role])
+ status = _execute(cur, u"SET ROLE TO %s", [mgr.role])
- if not status:
+ if status is not None:
self.conn.close()
self.conn = None
current_app.logger.error("""
@@ -337,35 +379,38 @@ Connect to the database server (#{server_id}) for connection ({conn_id}), but -
""".format(
server_id=self.manager.sid,
conn_id=conn_id,
- msg=res
+ msg=status
)
)
return False, \
_("Failed to setup the role with error message:\n{0}").format(
- res
+ status
)
if mgr.ver is None:
- status, res = self.execute_scalar("SELECT version()")
+ status = _execute(cur, "SELECT version()")
- if status:
- mgr.ver = res
- mgr.sversion = pg_conn.server_version
- else:
+ if status is not None:
self.conn.close()
self.conn = None
+ self.wasConneted = False
current_app.logger.error("""
Failed to fetch the version information on the established connection to the database server (#{server_id}) for '{conn_id}' with below error message:
{msg}
""".format(
server_id=self.manager.sid,
conn_id=conn_id,
- msg=res
+ msg=status
)
)
- return False, res
+ return False, status
+
+ if cur.rowcount > 0:
+ row = cur.fetchmany(1)[0]
+ mgr.ver = row['version']
+ mgr.sversion = self.conn.server_version
- status, res = self.execute_dict("""
+ status = _execute(cur, """
SELECT
db.oid as did, db.datname, db.datallowconn, pg_encoding_to_char(db.encoding) AS serverencoding,
has_database_privilege(db.oid, 'CREATE') as cancreate, datlastsysoid
@@ -373,16 +418,17 @@ FROM
pg_database db
WHERE db.datname = current_database()""")
- if status:
+ if status is None:
mgr.db_info = mgr.db_info or dict()
- f_row = res['rows'][0]
- mgr.db_info[f_row['did']] = f_row.copy()
+ if cur.rowcount > 0:
+ res = cur.fetchmany(1)[0]
+ mgr.db_info[res['did']] = res.copy()
- # We do not have database oid for the maintenance database.
- if len(mgr.db_info) == 1:
- mgr.did = f_row['did']
+ # We do not have database oid for the maintenance database.
+ if len(mgr.db_info) == 1:
+ mgr.did = res['did']
- status, res = self.execute_dict("""
+ status = _execute(cur, """
SELECT
oid as id, rolname as name, rolsuper as is_superuser,
rolcreaterole as can_create_role, rolcreatedb as can_create_db
@@ -391,28 +437,34 @@ FROM
WHERE
rolname = current_user""")
- if status:
+ if status is None:
mgr.user_info = dict()
- f_row = res['rows'][0]
- mgr.user_info = f_row.copy()
+ if cur.rowcount > 0:
+ mgr.user_info = cur.fetchmany(1)[0]
if 'password' in kwargs:
mgr.password = kwargs['password']
+ server_types = None
if 'server_types' in kwargs and isinstance(kwargs['server_types'], list):
- for st in kwargs['server_types']:
- if st.instanceOf(mgr.ver):
- mgr.server_type = st.stype
- mgr.server_cls = st
- break
+ server_types = mgr.server_types = kwargs['server_types']
+
+ if server_types is None:
+ from pgadmin.browser.server_groups.servers.types import ServerType
+ server_types = ServerType.types()
+
+ for st in server_types:
+ if st.instanceOf(mgr.ver):
+ mgr.server_type = st.stype
+ mgr.server_cls = st
+ break
- mgr._update_password(encpass)
mgr.update_session()
return True, None
def __cursor(self, server_cursor=False):
- if not self.conn:
+ if self.wasConnected is False:
raise ConnectionLost(
self.manager.sid,
self.db,
@@ -428,73 +480,68 @@ WHERE
return True, cur
if not self.connected():
- status = False
errmsg = ""
- current_app.logger.warning("""
-Connection to database server (#{server_id}) for the connection - '{conn_id}' has been lost.
-""".format(
- server_id=self.manager.sid,
- conn_id=self.conn_id
- )
+ current_app.logger.warning(
+ "Connection to database server (#{server_id}) for the connection - '{conn_id}' has been lost.".format(
+ server_id=self.manager.sid,
+ conn_id=self.conn_id
+ )
)
- if self.auto_reconnect:
- status, errmsg = self.connect()
-
- if not status:
- errmsg = gettext(
- """
-Attempt to reconnect failed with the error:
-{0}""".format(errmsg)
- )
-
- if not status:
- msg = gettext("Connection lost.\n{0}").format(errmsg)
- current_app.logger.error(errmsg)
-
- return False, msg
+ if self.auto_reconnect and not self.reconnecting:
+ self.__attempt_execution_reconnect(None)
+ else:
+ raise ConnectionLost(
+ self.manager.sid,
+ self.db,
+ None if self.conn_id[0:3] == u'DB:' else self.conn_id[5:]
+ )
try:
if server_cursor:
# Providing name to cursor will create server side cursor.
cursor_name = "CURSOR:{0}".format(self.conn_id)
- cur = self.conn.cursor(name=cursor_name,
- cursor_factory=DictCursor)
+ cur = self.conn.cursor(
+ name=cursor_name, cursor_factory=DictCursor
+ )
else:
cur = self.conn.cursor(cursor_factory=DictCursor)
except psycopg2.Error as pe:
- errmsg = gettext("""
-Failed to create cursor for psycopg2 connection with error message for the \
-server#{1}:{2}:
-{0}""").format(str(pe), self.manager.sid, self.db)
- current_app.logger.error(errmsg)
- self.conn.close()
- self.conn = None
+ current_app.logger.exception(pe)
+ errmsg = gettext(
+ "Failed to create cursor for psycopg2 connection with error message for the server#{1}:{2}:\n{0}"
+ ).format(
+ str(pe), self.manager.sid, self.db
+ )
- if self.auto_reconnect:
- current_app.logger.debug("""
-Attempting to reconnect to the database server (#{server_id}) for the connection - '{conn_id}'.
-""".format(
- server_id=self.manager.sid,
- conn_id=self.conn_id
- )
- )
- status, cur = self.connect()
- if not status:
- msg = gettext(
- u"""
-Connection for server#{0} with database "{1}" was lost.
-Attempt to reconnect it failed with the error:
-{2}"""
- ).format(self.driver.server_id, self.db, cur)
- current_app.logger.error(msg)
-
- return False, cur
- else:
- return False, errmsg
+ current_app.logger.error(errmsg)
+ if self.conn.closed:
+ self.conn = None
+ if self.auto_reconnect and not self.reconnecting:
+ current_app.logger.info(
+ gettext(
+ "Attempting to reconnect to the database server (#{server_id}) for the connection - '{conn_id}'."
+ ).format(
+ server_id=self.manager.sid,
+ conn_id=self.conn_id
+ )
+ )
+ return self.__attempt_execution_reconnect(
+ self.__cursor, server_cursor
+ )
+ else:
+ raise ConnectionLost(
+ self.manager.sid,
+ self.db,
+ None if self.conn_id[0:3] == u'DB:' else self.conn_id[5:]
+ )
- setattr(g, "{0}#{1}".format(self.manager.sid, self.conn_id.encode('utf-8')), cur)
+ setattr(
+ g, "{0}#{1}".format(
+ self.manager.sid, self.conn_id.encode('utf-8')
+ ), cur
+ )
return True, cur
@@ -537,7 +584,7 @@ Attempt to reconnect it failed with the error:
cur.close()
errmsg = self._formatted_exception_msg(pe, formatted_exception_msg)
current_app.logger.error(
- u"Failed to execute query ((with server cursor) for the server #{server_id} - {conn_id} (Query-id: {query_id}):\nError Message:{errmsg}".format(
+ u"failed to execute query ((with server cursor) for the server #{server_id} - {conn_id} (query-id: {query_id}):\nerror message:{errmsg}".format(
server_id=self.manager.sid,
conn_id=self.conn_id,
query=query,
@@ -606,6 +653,10 @@ Attempt to reconnect it failed with the error:
except psycopg2.Error as pe:
cur.close()
if not self.connected():
+ if self.auto_reconnect and not self.reconnecting:
+ return self.__attempt_execution_reconnect(
+ self.execute_dict, query, params, formatted_exception_msg
+ )
raise ConnectionLost(
self.manager.sid,
self.db,
@@ -713,6 +764,10 @@ Failed to execute query (execute_async) for the server #{server_id} - {conn_id}
except psycopg2.Error as pe:
cur.close()
if not self.connected():
+ if self.auto_reconnect and not self.reconnecting:
+ return self.__attempt_execution_reconnect(
+ self.execute_void, query, params, formatted_exception_msg
+ )
raise ConnectionLost(
self.manager.sid,
self.db,
@@ -736,6 +791,36 @@ Failed to execute query (execute_void) for the server #{server_id} - {conn_id}
return True, None
+ def __attempt_execution_reconnect(self, fn, *args, **kwargs):
+ self.reconnecting = True
+ setattr(g, "{0}#{1}".format(
+ self.manager.sid,
+ self.conn_id.encode('utf-8')
+ ), None)
+ try:
+ status, res = self.connect()
+ if status:
+ if fn:
+ status, res = fn(*args, **kwargs)
+ self.reconnecting = False
+ return status, res
+ except Exception as e:
+ current_app.logger.exception(e)
+ self.reconnecting = False
+
+ current_app.warning(
+ "Failed to reconnect the database server (#{server_id})".format(
+ server_id=self.manager.sid,
+ conn_id=self.conn_id
+ )
+ )
+ self.reconnecting = False
+ raise ConnectionLost(
+ self.manager.sid,
+ self.db,
+ None if self.conn_id[0:3] == u'DB:' else self.conn_id[5:]
+ )
+
def execute_2darray(self, query, params=None, formatted_exception_msg=False):
status, cur = self.__cursor()
self.row_count = 0
@@ -758,11 +843,11 @@ Failed to execute query (execute_void) for the server #{server_id} - {conn_id}
except psycopg2.Error as pe:
cur.close()
if not self.connected():
- raise ConnectionLost(
- self.manager.sid,
- self.db,
- None if self.conn_id[0:3] == u'DB:' else self.conn_id[5:]
- )
+ if self.wasConnected and self.auto_reconnect and \
+ not self.reconnecting:
+ return self.__attempt_execution_reconnect(
+ self.execute_2darray, query, params, formatted_exception_msg
+ )
errmsg = self._formatted_exception_msg(pe, formatted_exception_msg)
current_app.logger.error(
u"Failed to execute query (execute_2darray) for the server #{server_id} - {conn_id} (Query-id: {query_id}):\nError Message:{errmsg}".format(
@@ -809,6 +894,11 @@ Failed to execute query (execute_void) for the server #{server_id} - {conn_id}
except psycopg2.Error as pe:
cur.close()
if not self.connected():
+ if self.auto_reconnect and not self.reconnecting:
+ return self.__attempt_execution_reconnect(
+ self.execute_dict, query, params,
+ formatted_exception_msg
+ )
raise ConnectionLost(
self.manager.sid,
self.db,
@@ -900,10 +990,12 @@ Failed to reset the connection to the server due to following error:
return self.execute_scalar('SELECT 1')
def _release(self):
- if self.conn:
- self.conn.close()
- self.conn = None
+ if self.wasConneted:
+ if self.conn:
+ self.conn.close()
+ self.conn = None
self.password = None
+ self.wasConnected = False
def _wait(self, conn):
"""
@@ -1227,6 +1319,7 @@ class ServerManager(object):
self.ssl_mode = server.ssl_mode
self.pinged = datetime.datetime.now()
self.db_info = dict()
+ self.server_types = None
for con in self.connections:
self.connections[con]._release()
@@ -1430,7 +1523,7 @@ WHERE db.oid = {0}""".format(did))
self.password = passwd
for conn_id in self.connections:
conn = self.connections[conn_id]
- if conn.conn is not None:
+ if conn.conn is not None or conn.wasConnected is True:
conn.password = passwd
def update_session(self):
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: RM#1387 [Add-on PATCH] Bad handling of missing connection database server
In-Reply-To: <CAG7mmoxUL-H1poTGZz4KyMd9Tw3Po4yRDw_GK1Wu6w4YTK1WXA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox