Re: [pgAdmin4][Patch]: Allow user to cancel long running queries from dashboard

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Shirley Wang <swang(at)pivotal(dot)io>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Allow user to cancel long running queries from dashboard
Date: 2017-07-24 17:27:44
Message-ID: CAFiP3vwE0JDqym4+-PreLm4mrsCSNQ7t+bU-pvWpC=FWkXvRig@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

--
*Harshal Dhumal*
*Sr. Software Engineer*

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Jul 24, 2017 at 8:11 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Mon, Jul 24, 2017 at 3:28 PM, Shirley Wang <swang(at)pivotal(dot)io> wrote:
>
>> 2-3 days is a lot of valuable engineering time. Is this a 'drop
>> everything now' kind of feature or can this wait for some user validation
>> on a mock up first?
>>
>
> Most of the time will likely be on the infrastructure to change the
> display to a subnode control. If you have some cycles to mockup potential
> layouts for the subnode view and have them validated, please feel free,
> however, that seems like an awful lot of work to me to display some missing
> SQL using a standard control.
>
Regarding SQL display: Developing simple control to show codemirror in
disabled state (for now) wont take that much time.

>
>
>>
>> On Jul 24, 2017, at 20:41, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>> Sure.
>>
>> RM created: https://redmine.postgresql.org/issues/2597.
>>
>> On Mon, Jul 24, 2017 at 6:04 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Mon, Jul 24, 2017 at 1:12 PM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Jul 24, 2017 at 4:44 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Thanks, applied.
>>>>>
>>>>> How much effort do you think it would take to turn the table into a
>>>>> subnode control so we can show the SQL query and other missing fields from
>>>>> pg_stat_activity on that tab?
>>>>>
>>>> I think around 2-3 days would be needed.
>>>>
>>>
>>> OK - can you add a Redmine and work on that please? It's been a known
>>> deficiency for quite a while, and whilst working on this patch I realised
>>> just how badly we really need to see the SQL there.
>>>
>>> Thanks.
>>>
>>>
>>>>
>>>>>
>>>> On Mon, Jul 24, 2017 at 11:45 AM, Murtuza Zabuawala <
>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> Please find updated patch.
>>>>>>
>>>>>> On Tue, Jul 18, 2017 at 8:05 PM, Murtuza Zabuawala <
>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Shirley,
>>>>>>>
>>>>>>> On Tue, Jul 18, 2017 at 1:21 AM, Shirley Wang <swang(at)pivotal(dot)io>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>> I can't seem to get the patch to completely work on my computer,
>>>>>>>> only the close icon shows up plus the dialog and success/error messages,
>>>>>>>> but here are some comments:
>>>>>>>>
>>>>>>>> Because we are just cancelling the active running query, so if the
>>>>>>> start of the session is 'Active' when you cancel it will simply goto 'Idle'
>>>>>>> stat.
>>>>>>>
>>>>>>>> +1 to Dave's comment about refreshing after the cancel operation
>>>>>>>>
>>>>>>>> I'll fix this.
>>>>>>>
>>>>>>>> - We're working on a patch for updating alerts in the Dashboard tab
>>>>>>>> which updates the grays in the Database activities panel and changes the
>>>>>>>> border around the refresh button and search bar to 1px. This hasn't been
>>>>>>>> submitted yet but just a heads up as you work on the alignment.
>>>>>>>>
>>>>>>>> - Something to consider is how a super user will identify which
>>>>>>>> session should be closed. Is that information there?
>>>>>>>>
>>>>>>>> I think super user can cancel everything except main connection
>>>>>>> session & as Dave mentioned in previous email that background workers in
>>>>>>> PG10.
>>>>>>>
>>>>>>>> - Are there sessions that should never be closed? If so, do they
>>>>>>>> also need close buttons? (Probably not, because that will lead the user to
>>>>>>>> an error message, which is not fun)
>>>>>>>>
>>>>>>>> In Backgrid, we can not exclude specific column from certain rows
>>>>>>> if it renders in one row then it will render for every row in the grid,
>>>>>>> What we can do is, it will throw an error when user is not eligible to
>>>>>>> cancel the active running query.
>>>>>>>
>>>>>>>> - Perhaps this is a good feature to review with Chethana! :)
>>>>>>>>
>>>>>>>> On Mon, Jul 17, 2017 at 5:37 AM Murtuza Zabuawala <
>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> On Mon, Jul 17, 2017 at 3:01 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> On Thu, Jul 13, 2017 at 2:53 PM, Murtuza Zabuawala <
>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> hiHi,
>>>>>>>>>>>
>>>>>>>>>>> PFA patch to add functionality which will allow super user
>>>>>>>>>>> to cancel long running queries from dashboard.
>>>>>>>>>>> RM#1812
>>>>>>>>>>>
>>>>>>>>>>> *Steps used to test:*
>>>>>>>>>>> 1) Open psql session, Connect to 'test' database on respective
>>>>>>>>>>> server
>>>>>>>>>>> 2) Execute "select pg_sleep(1000);"
>>>>>>>>>>> 3) Open pgAdmin4
>>>>>>>>>>> 4) Connect to respective server
>>>>>>>>>>> 5) Click on Dashboard
>>>>>>>>>>> 6) Check "Sessions" tab under "Server activity" section then
>>>>>>>>>>> look for active sessions for test database.
>>>>>>>>>>> 7) Click on cancel button and cancel the active session
>>>>>>>>>>> 8) Check psql session now, you will see "ERROR: canceling
>>>>>>>>>>> statement due to user request"
>>>>>>>>>>>
>>>>>>>>>>> Some comments:
>>>>>>>>>>
>>>>>>>>>> - The action here is to cancel the active query in the backend,
>>>>>>>>>> not the session - so messages etc. should say things like "Cancel Active
>>>>>>>>>> Query?"
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> - The grid should refresh following the cancel operation.
>>>>>>>>>>
>>>>>>>>>> - Can you fix the vertical alignment while you're working on
>>>>>>>>>> this? The new button really makes the poor alignment stand out.
>>>>>>>>>>
>>>>>>>>>> - This should not be superuser only - regular users should be
>>>>>>>>>> able to cancel their own queries.
>>>>>>>>>>
>>>>>>>>>> - On PG10, background workers are also shown in the dashboard.
>>>>>>>>>> Should we prevent attempts to cancel their work (they'll fail anyway I
>>>>>>>>>> believe).
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Sure, I'll work on these comments & send updated patch.
>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> 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
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> 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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Shirley Wang 2017-07-25 05:26:31 Re: [pgAdmin4][Patch]: Allow user to cancel long running queries from dashboard
Previous Message Dave Page 2017-07-24 14:41:21 Re: [pgAdmin4][Patch]: Allow user to cancel long running queries from dashboard