Thread: PATCH: To fix the issue where message panel was showing incomplete info (pgAdmin4)

PATCH: To fix the issue where message panel was showing incomplete info (pgAdmin4)

From
Murtuza Zabuawala
Date:
Hi,

PFA patch to fix the issue where message panel was showing incomplete info.
We may still miss some messages from Psycopg2 driver due to limited size of notices queue.
(RM#1523)


Regards,
Murtuza

Attachment
Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?
Yeah - that's a good idea.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 
 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
 

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?
Yeah - that's a good idea.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 
 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

On Tue, Aug 9, 2016 at 6:34 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
That's a very huge delay in practical.
We were hardly waiting for during poll (that was in milliseconds), but - still we were loosing a lot of the messages. (a lot more from the current implementation).

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi

 

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?
Yeah - that's a good idea.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 
 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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



On Tue, Aug 9, 2016 at 2:07 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:34 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
That's a very huge delay in practical.
We were hardly waiting for during poll (that was in milliseconds), but - still we were loosing a lot of the messages. (a lot more from the current implementation).

What was the original delay? Now there appears to be none at all.
 

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi

 

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?
Yeah - that's a good idea.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 
 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
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

On Tue, Aug 9, 2016 at 6:47 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:07 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:34 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
That's a very huge delay in practical.
We were hardly waiting for during poll (that was in milliseconds), but - still we were loosing a lot of the messages. (a lot more from the current implementation).

What was the original delay? Now there appears to be none at all.
That was 10 milliseconds.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi

 

2) If I run VACUUM, whilst I see messages as I should now, there is no
query time summary as there is for a query that returns data.

3) Can we incrementally display messages? e.g. each time we poll, we
add the new messages to the messages div, and scroll to the end
automatically?
Yeah - that's a good idea.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


 
 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
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



On Tue, Aug 9, 2016 at 2:21 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:47 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:07 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:34 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
That's a very huge delay in practical.
We were hardly waiting for during poll (that was in milliseconds), but - still we were loosing a lot of the messages. (a lot more from the current implementation).

What was the original delay? Now there appears to be none at all.
That was 10 milliseconds

Hmm, Ok - for some reason I thought it was much longer. Ignore that point then :-)
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi Dave,

PFA patch with incremental messages.
RM#1523


Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Tue, Aug 9, 2016 at 6:58 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Aug 9, 2016 at 2:21 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:47 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:07 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:34 PM, Dave Page <dpage@pgadmin.org> wrote:



On Tue, Aug 9, 2016 at 2:01 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Tue, Aug 9, 2016 at 6:28 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Aug 9, 2016 at 8:07 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi,
>
> PFA patch to fix the issue where message panel was showing incomplete info.
> We may still miss some messages from Psycopg2 driver due to limited size of
> notices queue.
> (RM#1523)

A few thoughts on this (mostly based on my discussions with Ashesh):

1) You seem to have removed the poll delay. I assume that is to try to
avoid missing messages? Can we re-introduce the delay (to avoid
excessive network requests), but collect messages while we're waiting?
Using thread?
Start a thread during the timeout?

Not necessarily. If we want a 2 second polling delay, we could just sleep for 0.5 secs, collect messages, sleep for 0.5 sec, collect messages, <repeat...> return to client.
That's a very huge delay in practical.
We were hardly waiting for during poll (that was in milliseconds), but - still we were loosing a lot of the messages. (a lot more from the current implementation).

What was the original delay? Now there appears to be none at all.
That was 10 milliseconds

Hmm, Ok - for some reason I thought it was much longer. Ignore that point then :-)
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Attachment
Hi

On Tue, Aug 16, 2016 at 7:10 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> PFA patch with incremental messages.
> RM#1523

Can you rebase this please?

(pgadmin4)piranha:pgadmin4 dpage$ git apply
~/Downloads/RM_1523_with_incremental_msgs.patch
error: patch failed:
web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js:1296
error: web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js:
patch does not apply

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


Hi Dave,

PFA updated patch.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Tue, Aug 16, 2016 at 8:16 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Aug 16, 2016 at 7:10 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> PFA patch with incremental messages.
> RM#1523

Can you rebase this please?

(pgadmin4)piranha:pgadmin4 dpage$ git apply
~/Downloads/RM_1523_with_incremental_msgs.patch
error: patch failed:
web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js:1296
error: web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js:
patch does not apply

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Attachment
Hi

On Tue, Aug 16, 2016 at 3:56 PM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> PFA updated patch.

I just tested this, and found a number of issues:

- For a query like VACUUM FULL ANALYZE VERBOSE, the query runtime is
not displayed at the end of the messages. It is for SELECT queries
(note: pgAdmin 3 returns "Query returned successfully with no result
in 6.6 secs.").

- Messages seem to be duplicated. For example, the output from the
query above, run on pgAdmin3 is 95KB, whilst from pgAdmin4 it's
3.1MB(!). I believe it's because you're not clearing __notices in
messages(). I'm not entirely clear why that's even needed though -
can't we just return messages from self.conn.notices and clear that
directly?

- The messages div should scroll to the end as it's updated, per discussion.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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


Hi Dave,

Please find updated patch with auto scrolling & also fixed duplicate messages issue.
Please review.

Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Wed, Aug 17, 2016 at 7:09 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Aug 16, 2016 at 3:56 PM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> PFA updated patch.

I just tested this, and found a number of issues:

- For a query like VACUUM FULL ANALYZE VERBOSE, the query runtime is
not displayed at the end of the messages. It is for SELECT queries
(note: pgAdmin 3 returns "Query returned successfully with no result
in 6.6 secs.").

- Messages seem to be duplicated. For example, the output from the
query above, run on pgAdmin3 is 95KB, whilst from pgAdmin4 it's
3.1MB(!). I believe it's because you're not clearing __notices in
messages(). I'm not entirely clear why that's even needed though -
can't we just return messages from self.conn.notices and clear that
directly?

- The messages div should scroll to the end as it's updated, per discussion.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Attachment
Thanks, applied.

On Thu, Aug 18, 2016 at 10:21 AM, Murtuza Zabuawala
<murtuza.zabuawala@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find updated patch with auto scrolling & also fixed duplicate
> messages issue.
> Please review.
>
> Regards,
> Murtuza
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Aug 17, 2016 at 7:09 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Tue, Aug 16, 2016 at 3:56 PM, Murtuza Zabuawala
>> <murtuza.zabuawala@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > PFA updated patch.
>>
>> I just tested this, and found a number of issues:
>>
>> - For a query like VACUUM FULL ANALYZE VERBOSE, the query runtime is
>> not displayed at the end of the messages. It is for SELECT queries
>> (note: pgAdmin 3 returns "Query returned successfully with no result
>> in 6.6 secs.").
>>
>> - Messages seem to be duplicated. For example, the output from the
>> query above, run on pgAdmin3 is 95KB, whilst from pgAdmin4 it's
>> 3.1MB(!). I believe it's because you're not clearing __notices in
>> messages(). I'm not entirely clear why that's even needed though -
>> can't we just return messages from self.conn.notices and clear that
>> directly?
>>
>> - The messages div should scroll to the end as it's updated, per
>> discussion.
>>
>> Thanks.
>>
>> --
>> 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