Modifying data type of slot_keep_segs from XLogRecPtr to XLogSegNo
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.
How do you think?
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:
0001-change_type_of_slot_keep_segs.patchtext/x-diff; charset=us-ascii; name=0001-change_type_of_slot_keep_segs.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fd93bcfaeb..a8623cf996 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9603,7 +9603,7 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
/* Cap by max_slot_wal_keep_size ... */
if (max_slot_wal_keep_size_mb >= 0)
{
- XLogRecPtr slot_keep_segs;
+ XLogSegNo slot_keep_segs;
slot_keep_segs =
ConvertToXSegs(max_slot_wal_keep_size_mb, wal_segment_size);
On 2020/07/08 11:02, torikoshia wrote:
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.How do you think?
I agree that using XLogRecPtr for slot_keep_segs is incorrect.
But this variable indicates the number of segments rather than
segment no, uint64 seems better. Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On 2020-07-08 11:15, Fujii Masao wrote:
On 2020/07/08 11:02, torikoshia wrote:
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.How do you think?
I agree that using XLogRecPtr for slot_keep_segs is incorrect.
But this variable indicates the number of segments rather than
segment no, uint64 seems better. Thought?
That makes sense.
The number of segments and segment number are different.
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
On 2020/07/08 11:55, torikoshia wrote:
On 2020-07-08 11:15, Fujii Masao wrote:
On 2020/07/08 11:02, torikoshia wrote:
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.How do you think?
I agree that using XLogRecPtr for slot_keep_segs is incorrect.
But this variable indicates the number of segments rather than
segment no, uint64 seems better. Thought?That makes sense.
The number of segments and segment number are different.
Yes, so patch attached. I will commit it later.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
num_of_wal_segs.patchtext/plain; charset=UTF-8; name=num_of_wal_segs.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c2feb92576..3ebf8caee9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9601,7 +9601,7 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo)
/* Cap by max_slot_wal_keep_size ... */
if (max_slot_wal_keep_size_mb >= 0)
{
- XLogRecPtr slot_keep_segs;
+ uint64 slot_keep_segs;
slot_keep_segs =
ConvertToXSegs(max_slot_wal_keep_size_mb, wal_segment_size);
On 2020/07/08 15:22, Fujii Masao wrote:
On 2020/07/08 11:55, torikoshia wrote:
On 2020-07-08 11:15, Fujii Masao wrote:
On 2020/07/08 11:02, torikoshia wrote:
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.How do you think?
I agree that using XLogRecPtr for slot_keep_segs is incorrect.
But this variable indicates the number of segments rather than
segment no, uint64 seems better. Thought?That makes sense.
The number of segments and segment number are different.Yes, so patch attached. I will commit it later.
Pushed. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Wed, 8 Jul 2020 21:27:04 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2020/07/08 15:22, Fujii Masao wrote:
On 2020/07/08 11:55, torikoshia wrote:
On 2020-07-08 11:15, Fujii Masao wrote:
On 2020/07/08 11:02, torikoshia wrote:
Hi,
Currently, slot_keep_segs is defined as "XLogRecPtr" in KeepLogSeg(),
but it seems that should be "XLogSegNo" because this variable is
segment number.How do you think?
Yeah, that's my mistake while made bouncing back and forth between
segments and LSN in the code. I noticed that once but forgotten until
now. Thanks for finding it.
I agree that using XLogRecPtr for slot_keep_segs is incorrect.
But this variable indicates the number of segments rather than
segment no, uint64 seems better. Thought?That makes sense.
The number of segments and segment number are different.Yes, so patch attached. I will commit it later.
Pushed. Thanks!
Thanks!
--
Kyotaro Horiguchi
NTT Open Source Software Center