On Thu, Mar 16, 2017 at 8:28 AM, Rahila Syed <rahilasye...@gmail.com> wrote: > Thank you for the updated patch. > > I have applied and tested it on latest sources and the patch looks good to > me.
The documentation puts the new wait events in a pretty random order. I think they should be alphabetized, like we do with the IPC events. I also suggest we change the naming scheme so that the kind of thing being operated on is first and this is followed by the operation name. This will let us keep related entries next to each other after alphabetizing. So with that principle in mind: - instead of ReadDataBlock etc. I propose DataFileRead, DataFileWrite, DataFileSync, DataFileExtend, DataFileFlush, DataFilePrefetch, DataFileTruncate. using file instead of block avoids singular/plural confusion. - instead of RelationSync and RelationImmedSync I proposed DataFileSync and DataFileImmediateSync; these are md.c operations like the previous set, so why name it differently? - instead of WriteRewriteDataBlock and SyncRewriteDataBlock and TruncateLogicalMappingRewrite, which aren't consistent with each other even though they are related, I propose LogicalRewriteWrite, LogicalRewriteSync, and LogicalRewriteTruncate, which are also closer to the names of the functions that contain those wait points - for ReadBuffile and WriteBuffile seem OK, I propose BufFileRead and BufFileWrite, again reversing the order and also tweaking the capitalization - in keeping with our new policy of referring to xlog as wal in user visible interfaces, I propose WALRead, WALCopyRead, WALWrite, WALInitWrite, WALCopyWrite, WALBootstrapWrite, WALInitSync, WALBootstrapSync, WALSyncMethodAssign - for control file ops, ControlFileRead, ControlFileWrite, ControlFileWriteUpdate, ControlFileSync, ControlFileSyncUpdate - ReadApplyLogicalMapping and friends seem to have to do with the reorderbuffer code, so maybe ReorderBufferRead etc. - there seems to be some discrepancy between the documentation and pgstat_get_wait_io for the snapbuild stuff. maybe SnapBuildWrite, SnapBuildSync, SnapBuildRead. - SLRURead, SLRUWrite, etc. - TimelineHistoryRead, etc. - the syslogger changes should be dropped, since the syslogger is not and should not be connected to shared memory - the replslot terminology seems like a case of odd capitalization and overeager abbreviation. why not ReplicationSlotRead, ReplicationSlotWrite, etc? similarly RelationMapRead, RelationMapWrite, etc? - CopyFileRead, CopyFileWrite - LockFileCreateRead, etc. - AddToDataDirLockFileRead is a little long and incomprehensible; maybe LockFileUpdateRead etc. - DSMWriteZeroBytes, maybe? Of course the constants should be renamed to match. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
