Skip to content

Commit

Permalink
Fix an issue where queries longer than 1 minute get stuck - Container…
Browse files Browse the repository at this point in the history
… 7.1. #6317 (#6491)

Fix an issue where queries longer than 1 minute get stuck - Container 7.1. #6317
Fix an issue where queries get stuck with auto-completion enabled. #6356
Fix an issue where queries can't complete execution. #6163
  • Loading branch information
khushboovashi authored Jul 4, 2023
1 parent cba689d commit 326dc2b
Show file tree
Hide file tree
Showing 16 changed files with 245 additions and 267 deletions.
1 change: 0 additions & 1 deletion docs/en_US/release_notes_7_4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,3 @@ Bug fixes
| `Issue #6420 <https://github.com/pgadmin-org/pgadmin4/issues/6420>`_ - Fix raise notice from func/proc or code blocks are no longer displayed live.
| `Issue #6431 <https://github.com/pgadmin-org/pgadmin4/issues/6431>`_ - Fix an issue where PSQL is not working if the database name have quotes or double quotes.
| `Issue #6435 <https://github.com/pgadmin-org/pgadmin4/issues/6435>`_ - Fix an issue where all the menus are enabled when pgAdmin is opened and no object is selected in the object explorer.
3 changes: 3 additions & 0 deletions docs/en_US/release_notes_7_5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ Housekeeping
Bug fixes
*********

| `Issue #6163 <https://github.com/pgadmin-org/pgadmin4/issues/6163>`_ - Fix an issue where queries can't complete execution.
| `Issue #6165 <https://github.com/pgadmin-org/pgadmin4/issues/6165>`_ - Fixed an issue where Import Export not working when using pgpassfile.
| `Issue #6317 <https://github.com/pgadmin-org/pgadmin4/issues/6317>`_ - Fix an issue where queries longer than 1 minute get stuck - Container 7.1
| `Issue #6356 <https://github.com/pgadmin-org/pgadmin4/issues/6356>`_ - Fix an issue where queries get stuck with auto-completion enabled.
| `Issue #6364 <https://github.com/pgadmin-org/pgadmin4/issues/6364>`_ - Fixed Query Tool/ PSQL tool tab title not getting updated on database rename.
| `Issue #6515 <https://github.com/pgadmin-org/pgadmin4/issues/6515>`_ - Fixed an issue where the query tool is unable to execute a query on Postgres 10 and below versions.
108 changes: 82 additions & 26 deletions web/pgadmin/tools/sqleditor/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import secrets
from urllib.parse import unquote
from threading import Lock
import threading

import json
from config import PG_DEFAULT_DRIVER, ALLOW_SAVE_PASSWORD, SHARED_STORAGE
Expand All @@ -34,7 +35,7 @@
from pgadmin.utils import PgAdminModule
from pgadmin.utils import get_storage_directory
from pgadmin.utils.ajax import make_json_response, bad_request, \
success_return, internal_server_error
success_return, internal_server_error, service_unavailable
from pgadmin.utils.driver import get_driver
from pgadmin.utils.exception import ConnectionLost, SSHTunnelConnectionLost, \
CryptKeyMissing, ObjectGone
Expand Down Expand Up @@ -415,14 +416,16 @@ def _connect(conn, **kwargs):
def _init_sqleditor(trans_id, connect, sgid, sid, did, **kwargs):
# Create asynchronous connection using random connection id.
conn_id = str(secrets.choice(range(1, 9999999)))
conn_id_ac = str(secrets.choice(range(1, 9999999)))

manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)

if did is None:
did = manager.did
try:
command_obj = ObjectRegistry.get_object(
'query_tool', conn_id=conn_id, sgid=sgid, sid=sid, did=did
'query_tool', conn_id=conn_id, sgid=sgid, sid=sid, did=did,
conn_id_ac=conn_id_ac
)
except Exception as e:
current_app.logger.error(e)
Expand All @@ -433,8 +436,8 @@ def _init_sqleditor(trans_id, connect, sgid, sid, did, **kwargs):
auto_reconnect=False,
use_binary_placeholder=True,
array_to_string=True)

pref = Preferences.module('sqleditor')

if connect:
kwargs['auto_commit'] = pref.preference('auto_commit').get()
kwargs['auto_rollback'] = pref.preference('auto_rollback').get()
Expand All @@ -461,6 +464,15 @@ def _init_sqleditor(trans_id, connect, sgid, sid, did, **kwargs):
else:
return True, internal_server_error(
errormsg=str(msg)), '', ''

if pref.preference('autocomplete_on_key_press').get():
conn_ac = manager.connection(did=did, conn_id=conn_id_ac,
auto_reconnect=False,
use_binary_placeholder=True,
array_to_string=True)
status, msg, is_ask_password, user, role, password = _connect(
conn_ac, **kwargs)

except (ConnectionLost, SSHTunnelConnectionLost) as e:
current_app.logger.error(e)
raise
Expand Down Expand Up @@ -659,15 +671,30 @@ def close_sqleditor_session(trans_id):
conn.cancel_transaction(cmd_obj.conn_id, cmd_obj.did)
manager.release(did=cmd_obj.did, conn_id=cmd_obj.conn_id)

# Close the auto complete connection
if cmd_obj.conn_id_ac is not None:
manager = get_driver(
PG_DEFAULT_DRIVER).connection_manager(cmd_obj.sid)
if manager is not None:
conn = manager.connection(
did=cmd_obj.did, conn_id=cmd_obj.conn_id_ac)

# Release the connection
if conn.connected():
conn.cancel_transaction(cmd_obj.conn_id_ac, cmd_obj.did)
manager.release(did=cmd_obj.did,
conn_id=cmd_obj.conn_id_ac)


def check_transaction_status(trans_id):
def check_transaction_status(trans_id, auto_comp=False):
"""
This function is used to check the transaction id
is available in the session object and connection
status.
Args:
trans_id:
trans_id: Transaction Id
auto_comp: Auto complete flag
Returns: status and connection object
Expand All @@ -687,12 +714,19 @@ def check_transaction_status(trans_id):
session_obj = grid_data[str(trans_id)]
trans_obj = pickle.loads(session_obj['command_obj'])

if auto_comp:
conn_id = trans_obj.conn_id_ac
connect = True
else:
conn_id = trans_obj.conn_id
connect = True if 'connect' in request.args and \
request.args['connect'] == '1' else False
try:
manager = get_driver(
PG_DEFAULT_DRIVER).connection_manager(trans_obj.sid)
conn = manager.connection(
did=trans_obj.did,
conn_id=trans_obj.conn_id,
conn_id=conn_id,
auto_reconnect=False,
use_binary_placeholder=True,
array_to_string=True
Expand All @@ -703,10 +737,7 @@ def check_transaction_status(trans_id):
current_app.logger.error(e)
return False, internal_server_error(errormsg=str(e)), None, None, None

connect = True if 'connect' in request.args and \
request.args['connect'] == '1' else False

if connect:
if connect and conn and not conn.connected():
conn.connect()

return True, None, conn, trans_obj, session_obj
Expand Down Expand Up @@ -874,23 +905,38 @@ def poll(trans_id):
data_obj = {}
on_demand_record_count = Preferences.module(MODULE_NAME).\
preference('on_demand_record_count').get()

# Check the transaction and connection status
status, error_msg, conn, trans_obj, session_obj = \
check_transaction_status(trans_id)

if type(error_msg) is Response:
return error_msg

if error_msg == ERROR_MSG_TRANS_ID_NOT_FOUND:
return make_json_response(success=0, errormsg=error_msg,
info='DATAGRID_TRANSACTION_REQUIRED',
status=404)

if not conn.async_cursor_initialised():
return make_json_response(data={'status': 'NotInitialised'})
is_thread_alive = False
if trans_obj.get_thread_native_id():
for thread in threading.enumerate():
if thread.native_id == trans_obj.get_thread_native_id() and\
thread.is_alive():
is_thread_alive = True
break

if status and conn is not None and session_obj is not None:
if is_thread_alive:
status = 'Busy'
elif status and conn is not None and session_obj is not None:
status, result = conn.poll(
formatted_exception_msg=True, no_result=True)
if not status:
if not conn.connected():
return service_unavailable(
gettext("Connection to the server has been lost."),
info="CONNECTION_LOST",
)

messages = conn.messages()
if messages and len(messages) > 0:
additional_messages = ''.join(messages)
Expand Down Expand Up @@ -1029,8 +1075,9 @@ def poll(trans_id):
status = 'NotConnected'
result = error_msg

transaction_status = conn.transaction_status()
data_obj['db_name'] = conn.db
transaction_status = conn.transaction_status() if conn else 0
data_obj['db_name'] = conn.db if conn else None

data_obj['db_id'] = trans_obj.did \
if trans_obj is not None and hasattr(trans_obj, 'did') else 0

Expand Down Expand Up @@ -1760,7 +1807,7 @@ def auto_complete(trans_id):

# Check the transaction and connection status
status, error_msg, conn, trans_obj, session_obj = \
check_transaction_status(trans_id)
check_transaction_status(trans_id, auto_comp=True)

if error_msg == ERROR_MSG_TRANS_ID_NOT_FOUND:
return make_json_response(success=0, errormsg=error_msg,
Expand All @@ -1770,15 +1817,18 @@ def auto_complete(trans_id):
if status and conn is not None and \
trans_obj is not None and session_obj is not None:

if trans_id not in auto_complete_objects:
# Create object of SQLAutoComplete class and pass connection object
auto_complete_objects[trans_id] = \
SQLAutoComplete(sid=trans_obj.sid, did=trans_obj.did,
conn=conn)

auto_complete_obj = auto_complete_objects[trans_id]
# Get the auto completion suggestions.
res = auto_complete_obj.get_completions(full_sql, text_before_cursor)
with sqleditor_close_session_lock:
if trans_id not in auto_complete_objects:
# Create object of SQLAutoComplete class and pass
# connection object
auto_complete_objects[trans_id] = \
SQLAutoComplete(sid=trans_obj.sid, did=trans_obj.did,
conn=conn)

auto_complete_obj = auto_complete_objects[trans_id]
# # Get the auto completion suggestions.
res = auto_complete_obj.get_completions(full_sql,
text_before_cursor)
else:
status = False
res = error_msg
Expand Down Expand Up @@ -2455,6 +2505,12 @@ def add_query_history(trans_id):
status, error_msg, conn, trans_obj, session_ob = \
check_transaction_status(trans_id)

if not trans_obj:
return make_json_response(
data={
'status': False,
}
)
return QueryHistory.save(current_user.id, trans_obj.sid, conn.db,
request=request)

Expand Down
20 changes: 20 additions & 0 deletions web/pgadmin/tools/sqleditor/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ def __init__(self, **kwargs):
if self.cmd_type in (VIEW_FIRST_100_ROWS, VIEW_LAST_100_ROWS):
self.limit = 100

self.thread_native_id = None

def get_primary_keys(self, *args, **kwargs):
return None, None

Expand Down Expand Up @@ -441,6 +443,12 @@ def get_pk_order(self):
else:
return 'asc'

def get_thread_native_id(self):
return self.thread_native_id

def set_thread_native_id(self, thread_native_id):
self.thread_native_id = thread_native_id


class TableCommand(GridCommand):
"""
Expand Down Expand Up @@ -881,6 +889,8 @@ def __init__(self, **kwargs):
FetchedRowTracker.__init__(self, **kwargs)

self.conn_id = kwargs['conn_id'] if 'conn_id' in kwargs else None
self.conn_id_ac = kwargs['conn_id_ac'] if 'conn_id_ac' in kwargs\
else None
self.auto_rollback = False
self.auto_commit = True

Expand All @@ -890,6 +900,7 @@ def __init__(self, **kwargs):
self.pk_names = None
self.table_has_oids = False
self.columns_types = None
self.thread_native_id = None

def get_sql(self, default_conn=None):
return None
Expand Down Expand Up @@ -982,6 +993,9 @@ def save(self,
def set_connection_id(self, conn_id):
self.conn_id = conn_id

def set_connection_id_ac(self, conn_id):
self.conn_id_ac = conn_id

def set_auto_rollback(self, auto_rollback):
self.auto_rollback = auto_rollback

Expand Down Expand Up @@ -1009,3 +1023,9 @@ def __set_updatable_results_attrs(self, sql_path,
self.object_name = result['rows'][0]['relname']
else:
raise InternalServerError(SERVER_CONNECTION_CLOSED)

def get_thread_native_id(self):
return self.thread_native_id

def set_thread_native_id(self, thread_native_id):
self.thread_native_id = thread_native_id
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ export class ResultSetUtils {
return msg;
}

static isCursorInitialised(httpMessage) {
return httpMessage.data.data.status === 'NotInitialised';
}

static isQueryFinished(httpMessage) {
return httpMessage.data.data.status === 'Success';
}
Expand Down Expand Up @@ -282,7 +278,7 @@ export class ResultSetUtils {
});
}

handlePollError(error) {
handlePollError(error, explainObject, flags) {
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.EXECUTION_END);
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.FOCUS_PANEL, PANELS.MESSAGES);
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.SET_CONNECTION_STATUS, CONNECTION_STATUS.TRANSACTION_STATUS_INERROR);
Expand All @@ -296,28 +292,31 @@ export class ResultSetUtils {
query_source: this.historyQuerySource,
is_pgadmin_query: false,
});
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error);
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, {
connectionLostCallback: ()=>{
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.EXECUTION_START, this.query, explainObject, flags.external, true);
},
checkTransaction: true,
});
}

async pollForResult(onResultsAvailable, onExplain, onPollError) {
async pollForResult(onResultsAvailable, onExplain, onPollError, explainObject, flags) {
try {
let httpMessage = await this.poll();
let msg = '';
if(httpMessage.data.data.notifies) {
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.PUSH_NOTICE, httpMessage.data.data.notifies);
}

if (ResultSetUtils.isCursorInitialised(httpMessage)) {
return Promise.resolve(this.pollForResult(onResultsAvailable, onExplain, onPollError));
} else if (ResultSetUtils.isQueryFinished(httpMessage)) {
if (ResultSetUtils.isQueryFinished(httpMessage)) {
this.setEndTime(new Date());
msg = this.queryFinished(httpMessage, onResultsAvailable, onExplain);
} else if (ResultSetUtils.isQueryStillRunning(httpMessage)) {
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.SET_CONNECTION_STATUS, httpMessage.data.data.transaction_status);
if(httpMessage.data.data.result) {
this.eventBus.fireEvent(QUERY_TOOL_EVENTS.SET_MESSAGE, httpMessage.data.data.result, true);
}
return Promise.resolve(this.pollForResult(onResultsAvailable, onExplain, onPollError));
return Promise.resolve(this.pollForResult(onResultsAvailable, onExplain, onPollError, explainObject, flags));
} else if (ResultSetUtils.isConnectionToServerLostWhilePolling(httpMessage)) {
this.setEndTime(new Date());
msg = httpMessage.data.data.result;
Expand Down Expand Up @@ -348,7 +347,7 @@ export class ResultSetUtils {
}
} catch (error) {
onPollError();
this.handlePollError(error);
this.handlePollError(error, explainObject, flags);
}
}

Expand Down Expand Up @@ -830,12 +829,14 @@ export function ResultSet() {
()=>{
setColumns([]);
setRows([]);
}
},
explainObject,
{isQueryTool: queryToolCtx.params.is_query_tool, external: external, reconnect: reconnect}
);
};

const executeAndPoll = async ()=>{
yesCallback();
await yesCallback();
pollCallback();
};

Expand Down
Loading

0 comments on commit 326dc2b

Please sign in to comment.