Обсуждение: Why SyncRepWakeQueue is not static?
SyncRepWakeQueue (src/backend/replication/syncrep.c) is not used anywhere except in the file. If there's no good reason for it, I think it should be declared as a static function. Included patch does so. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index ec594cf..325239d 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -69,6 +69,7 @@ static int SyncRepWaitMode = SYNC_REP_NO_WAIT;static void SyncRepQueueInsert(int mode);static void SyncRepCancelWait(void); +static int SyncRepWakeQueue(bool all, int mode);static int SyncRepGetStandbyPriority(void); @@ -546,7 +547,7 @@ SyncRepGetStandbyPriority(void) * * Must hold SyncRepLock. */ -int +static intSyncRepWakeQueue(bool all, int mode){ volatile WalSndCtlData *walsndctl = WalSndCtl; diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h index b3d399d..71e2857 100644 --- a/src/include/replication/syncrep.h +++ b/src/include/replication/syncrep.h @@ -47,9 +47,6 @@ extern void SyncRepReleaseWaiters(void);/* called by checkpointer */extern void SyncRepUpdateSyncStandbysDefined(void); -/* called by various procs */ -extern int SyncRepWakeQueue(bool all, int mode); -/* forward declaration to avoid pulling in walsender_private.h */struct WalSnd;extern struct WalSnd *SyncRepGetSynchronousStandby(void);
On Wed, Mar 25, 2015 at 12:13 PM, Tatsuo Ishii <ishii@postgresql.org> wrote: > SyncRepWakeQueue (src/backend/replication/syncrep.c) is not used > anywhere except in the file. If there's no good reason for it, I think > it should be declared as a static function. Included patch does so. That's indeed contradictory with what is written in syncrep.h, the function is not called from other places. -- Michael
> SyncRepWakeQueue (src/backend/replication/syncrep.c) is not used > anywhere except in the file. If there's no good reason for it, I think > it should be declared as a static function. Included patch does so. Fix committed/pushed from master to 9.2. 9.1 declares it as a static function. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On Thu, Mar 26, 2015 at 10:46 AM, Tatsuo Ishii <ishii@postgresql.org> wrote: >> SyncRepWakeQueue (src/backend/replication/syncrep.c) is not used >> anywhere except in the file. If there's no good reason for it, I think >> it should be declared as a static function. Included patch does so. > > Fix committed/pushed from master to 9.2. 9.1 declares it as a static > function. Er, is that a good idea to back-patch that? Normally routine specs are maintained stable on back-branches, and this is just a cosmetic change. -- Michael
>> Fix committed/pushed from master to 9.2. 9.1 declares it as a static >> function. > > Er, is that a good idea to back-patch that? Normally routine specs are > maintained stable on back-branches, and this is just a cosmetic > change. I'm not sure if it's a cosmetic change or not. I thought declaring to-be-static function as extern is against our coding standard. Moreover, if someone wants to change near the place in the source code in the future, changes made to head may not be easily back patched or cherry-picked to older branches if I do not back patch it. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On Wed, Mar 25, 2015 at 10:07 PM, Tatsuo Ishii <ishii@postgresql.org> wrote: >>> Fix committed/pushed from master to 9.2. 9.1 declares it as a static >>> function. >> >> Er, is that a good idea to back-patch that? Normally routine specs are >> maintained stable on back-branches, and this is just a cosmetic >> change. > > I'm not sure if it's a cosmetic change or not. I thought declaring > to-be-static function as extern is against our coding > standard. Moreover, if someone wants to change near the place in the > source code in the future, changes made to head may not be easily back > patched or cherry-picked to older branches if I do not back patch it. True. But if any third-party code calls that function, you just broke it. I don't think keeping the back-branches consistent with master is a sufficiently good reason for such a change. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company