PS display and standby query conflict
Hi,
When I created the conflict between recovery and many read-only
transactions in the standby server for test purpose, I found that the
keyword "waiting" disappeared from PS display for just a moment
even though the conflict had not been resolved yet. This seems
strange to me.
This problem happens because ResolveRecoveryConflictWithVirtualXIDs
resets PS display for each read-only transactions that recovery
waits for. Why do we need to reset that each time even though
the conflict has not been resolved yet? The attached patch
suppresses such a needless reset. Comments?
BTW, ResolveRecoveryConflictWithVirtualXIDs calls
pgstat_report_waiting(), which is also needless since the startup
process doesn't have the shared memory entry (i.e., MyBEEntry)
for pg_stat_activity. The attached patch removes that call.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
ps_display_v1.patchapplication/octet-stream; name=ps_display_v1.patchDownload
*** a/src/backend/storage/ipc/standby.c
--- b/src/backend/storage/ipc/standby.c
***************
*** 21,27 ****
#include "access/xact.h"
#include "access/xlog.h"
#include "miscadmin.h"
- #include "pgstat.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
--- 21,26 ----
***************
*** 191,206 **** static void
ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
ProcSignalReason reason)
{
! while (VirtualTransactionIdIsValid(*waitlist))
! {
! TimestampTz waitStart;
! char *new_status;
!
! pgstat_report_waiting(true);
! waitStart = GetCurrentTimestamp();
! new_status = NULL; /* we haven't changed the ps display */
/* reset standbyWait_us for each xact we wait for */
standbyWait_us = STANDBY_INITIAL_WAIT_US;
--- 190,203 ----
ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
ProcSignalReason reason)
{
! TimestampTz waitStart;
! char *new_status;
! waitStart = GetCurrentTimestamp();
! new_status = NULL; /* we haven't changed the ps display */
+ while (VirtualTransactionIdIsValid(*waitlist))
+ {
/* reset standbyWait_us for each xact we wait for */
standbyWait_us = STANDBY_INITIAL_WAIT_US;
***************
*** 246,262 **** ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
}
}
- /* Reset ps display if we changed it */
- if (new_status)
- {
- set_ps_display(new_status, false);
- pfree(new_status);
- }
- pgstat_report_waiting(false);
-
/* The virtual transaction is gone now, wait for the next one */
waitlist++;
}
}
void
--- 243,258 ----
}
}
/* The virtual transaction is gone now, wait for the next one */
waitlist++;
}
+
+ /* Reset ps display if we changed it */
+ if (new_status)
+ {
+ set_ps_display(new_status, false);
+ pfree(new_status);
+ }
}
void
On Thu, 2010-12-09 at 22:13 +0900, Fujii Masao wrote:
When I created the conflict between recovery and many read-only
transactions in the standby server for test purpose, I found that the
keyword "waiting" disappeared from PS display for just a moment
even though the conflict had not been resolved yet. This seems
strange to me.This problem happens because ResolveRecoveryConflictWithVirtualXIDs
resets PS display for each read-only transactions that recovery
waits for. Why do we need to reset that each time even though
the conflict has not been resolved yet? The attached patch
suppresses such a needless reset. Comments?
The reset occurs at most each 500ms, so not much problem there.
But if it annoys you, it seems OK to change it. Don't see a reason to backpatch though?
BTW, ResolveRecoveryConflictWithVirtualXIDs calls
pgstat_report_waiting(), which is also needless since the startup
process doesn't have the shared memory entry (i.e., MyBEEntry)
for pg_stat_activity. The attached patch removes that call.
IIRC that wasn't added by me, so not sure why its there. Not harming anything either.
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
On Sat, Dec 11, 2010 at 11:28 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
This problem happens because ResolveRecoveryConflictWithVirtualXIDs
resets PS display for each read-only transactions that recovery
waits for. Why do we need to reset that each time even though
the conflict has not been resolved yet? The attached patch
suppresses such a needless reset. Comments?The reset occurs at most each 500ms, so not much problem there.
But if it annoys you, it seems OK to change it. Don't see a reason to backpatch though?
I think that It's worth backpatch to prevent users who observe the
occurrence of the query conflicts carefully for testing 9.0 from
getting confusing.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Sun, Dec 12, 2010 at 10:06 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
But if it annoys you, it seems OK to change it. Don't see a reason to backpatch though?
I think that It's worth backpatch to prevent users who observe the
occurrence of the query conflicts carefully for testing 9.0 from
getting confusing.
I don't think this is important enough to back-patch, but it does seem
like a good idea in general, so committed, but just to the master
branch.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company