Visibility map page pinned for too long ?

Started by Pavan Deolaseeabout 13 years ago2 messages
#1Pavan Deolasee
pavan.deolasee@gmail.com

I was looking at the code when the following tiny bit caught my attention.
In vacuumlazy.c, we release the pin on the final VM page at line number 972.

954 if (vacrelstats->num_dead_tuples > 0)
955 {
956 /* Log cleanup info before we touch indexes */
957 vacuum_log_cleanup_info(onerel, vacrelstats);
958
959 /* Remove index entries */
960 for (i = 0; i < nindexes; i++)
961 lazy_vacuum_index(Irel[i],
962 &indstats[i],
963 vacrelstats);
964 /* Remove tuples from heap */
965 lazy_vacuum_heap(onerel, vacrelstats);
966 vacrelstats->num_index_scans++;
967 }
968
969 /* Release the pin on the visibility map page */
970 if (BufferIsValid(vmbuffer))
971 {
972 ReleaseBuffer(vmbuffer);
973 vmbuffer = InvalidBuffer;
974 }

So we are holding the pin right through the index vacuuming and the second
pass over the heap; both can take a very long time. We can and should
really be releasing the pin *before* those steps. In fact, it would be
appropriate to do it right after the preceding big for-loop.

While it may or may not matter from the performance or correctness
perspective, I think we should fix that.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Pavan Deolasee (#1)
Re: Visibility map page pinned for too long ?

On 3 December 2012 17:37, Pavan Deolasee <pavan.deolasee@gmail.com> wrote:

I was looking at the code when the following tiny bit caught my attention.
In vacuumlazy.c, we release the pin on the final VM page at line number 972.

954 if (vacrelstats->num_dead_tuples > 0)
955 {
956 /* Log cleanup info before we touch indexes */
957 vacuum_log_cleanup_info(onerel, vacrelstats);
958
959 /* Remove index entries */
960 for (i = 0; i < nindexes; i++)
961 lazy_vacuum_index(Irel[i],
962 &indstats[i],
963 vacrelstats);
964 /* Remove tuples from heap */
965 lazy_vacuum_heap(onerel, vacrelstats);
966 vacrelstats->num_index_scans++;
967 }
968
969 /* Release the pin on the visibility map page */
970 if (BufferIsValid(vmbuffer))
971 {
972 ReleaseBuffer(vmbuffer);
973 vmbuffer = InvalidBuffer;
974 }

So we are holding the pin right through the index vacuuming and the second
pass over the heap; both can take a very long time. We can and should really
be releasing the pin *before* those steps. In fact, it would be appropriate
to do it right after the preceding big for-loop.

While it may or may not matter from the performance or correctness
perspective, I think we should fix that.

Yes, its a clear bug. Patched.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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