(My mailer seems to have recovered from unresponsiveness.)
At Tue, 24 Nov 2020 12:29:41 -0500, Tom Lane wrote in
> Kyotaro Horiguchi writes:
> > At Fri, 20 Nov 2020 15:57:46 -0500, Tom Lane wrote in
> >> I don't much like anything about float8_coef_mul().
>
>
At Wed, 25 Nov 2020 11:39:39 +0900 (JST), Kyotaro Horiguchi
wrote in
> > So that line of thought prompts me to tread *very* carefully when
> > trying to dodge NaN results. We need to be certain that we
> > introduce only logically-defensible special cases. Something like
&
be possible to modify DISCARD ALL to discard
> CachedPlan and run it periodically. However, we think the burden
> on the user is high.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
done a buffer mapping
> > > lookup, and we don't hold a partition lock!
> >
> > That's because the buffer partition lock is released immediately after the
> > hash
> > table has been looked up. As an aside, InvalidateBuffer() requires the
> > caller
> > to hold the buffer header spinlock and doesn't hold the buffer partition
> > lock.
>
> Yes. Holding the buffer header spinlock is necessary to invalidate the
> buffers.
> As for buffer mapping partition lock, as mentioned by Tsunakawa-san, it is
> released immediately after BufTableLookup, which is similar to lookup done in
> PrefetchSharedBuffer. So I retained these changes.
>
> I have attached the updated patches. Aside from descriptions, no other major
> changes in the patch set except 0004. Feedbacks are welcome.
FWIW, As tunakawa-san mentioned, the partition lock is release
immedately after the look-up. The reason that we may release the
partition lock immediately is that it is OK that the buffer has been
evicted by someone to reuse it for other relations. We can know that
case by rechecking the buffer tag after holding header lock.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
al_level=minimal without taking a new base backup.
>
> Lastly, this should be backpatched.
> Any comments ?
Perhaps we need the TAP test that conducts the above steps.
> [1]
> https://www.postgresql.org/message-id/TYAPR01MB29901EBE5A3ACCE55BA99186FE320%40TYAPR01MB2990.jpnprd01.prod.outlook.com
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Thu, 26 Nov 2020 16:18:55 +0900 (JST), Kyotaro Horiguchi
wrote in
> + /* Zero the array of blocks because these will all be dropped anyway */
> + MemSet(firstDelBlocks, 0, sizeof(BlockNumber) * n * (MAX_FORKNUM + 1));
>
> We don't need to prepare nforks, forks and f
ng, like
+ * remember more references in the same resource owner, to avoid
+* that.
+*/
If I read this patch correctly, ResourceOwnerForget doesn't seem to do
such a thing for hash?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
be rather confusing to users. So I'm now thinking
> not to update that.
>
> Does anyone object to the patch? If no, I'm thinking to commit the
> patch.
Although I don't object to make the parameter reloadable, I think it
needs to be documented that server could stop after relo
At Fri, 27 Nov 2020 09:48:25 +0900, Fujii Masao
wrote in
>
>
> On 2020/11/27 9:30, Kyotaro Horiguchi wrote:
> > At Thu, 26 Nov 2020 22:43:48 +0900, Fujii Masao
> > wrote in
> >>
> >>
> >> On 2020/11/12 4:38, Sergei Kornilov wrote:
> >&
At Fri, 27 Nov 2020 02:19:57 +, "k.jami...@fujitsu.com"
wrote in
> > From: Kyotaro Horiguchi
> > Hello, Kirk. Thank you for the new version.
>
> Hi, Horiguchi-san. Thank you for your very helpful feedback.
> I'm updating the patches addressing
{
+ XLogResetInsertion();
+ return GetXLogInsertRecPtr();
What is the reason for those kinds of records to be emitted?
I think we must emit at least the shutdown checkpoint record, which
has redo-LSN that points to the record itself. I'm not sure what
other kinds of records are needed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
ED
XLOG_XACT_ABORT_PREPARED
XLOG_XACT_ASSIGNMENT
XLOG_XACT_INVALIDATIONS
Do we need all of these?
And, currenly what decides whether to emit a wal record according to
wal_level is the caller of XLogInsert. So doing this at
XLogInsert-level means that we bring the criteria of the necessit
*, are emitted. What's the
> > good name? IIUC, "minimal" is named after the fact that the minimal
> > amount of WAL necessary for crash recovery is generated. "norecovery" or
> > "unrecoverable"?
> Lastly, I found another name which expresses the essential characteristic of
> this wal_level.
> How about the name of wal_level="crash_unsafe" ?
> What did you think ?
I don't dislike "none" since it seems to me practically "none". It
seems rather correct if we actually need only the shutdown checkpoint
record.
"unrecoverable" is apparently misleading. "crash_unsafe" is precise
but seems somewhat alien being among "logical", "replica" and
"minimal".
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Fri, 08 Jan 2021 17:52:21 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Fri, 08 Jan 2021 14:47:05 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > This version RelationChangePersistence() is changed not to choose
> > in-place method for indexes other than btree. It seems to
he way all
of this kind of test follow.
The last check on table content is actually useless but it might make
sense to confirm that replication is actually working. However, I
don't think the test don't need to insert as many as 1000 tuples. Just
a single tuple would suffice.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
is prevents future index AMs from being
pluggable. Providing an interface would work but seems a bit too
invasive. The record insertion flags is there for this very purpose.
XLogBeginInsert();
XLogSetRecrodFlags(XLOG_MARK_ESSENTIAL); # new flag value
XLOGInsert();
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 12 Jan 2021 08:49:53 +0530, Amit Kapila wrote
in
> On Fri, Jan 8, 2021 at 7:03 AM Kyotaro Horiguchi
> wrote:
> >
> > At Thu, 7 Jan 2021 09:25:22 +, "k.jami...@fujitsu.com"
> > wrote in:
> > > > Thanks for the detailed tests. NBuffers/
t works as expected accidentally, but the meaning is off.
WAL-skipping optmization is irrelevant to the condition for the error.
I found five misues in the tree. Please find the attached.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 2912e1db38ff034e27e4010eff5b2e5afcce3f85 Mon S
At Wed, 13 Jan 2021 06:01:34 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > XLogBeginInsert();
> > XLogSetRecrodFlags(XLOG_MARK_ESSENTIAL); # new flag value
> > XLOGInsert();
>
> Oh, sounds like a nice idea. That
At Wed, 13 Jan 2021 16:51:32 -0300, Alvaro Herrera
wrote in
> On 2021-Jan-13, Fujii Masao wrote:
>
> > Thanks for the review!
> > I'm ok with this change (i.e., insert only single row).
> > Attached is the updated version of the patch.
>
> Looks good to me,
At Thu, 14 Jan 2021 12:34:01 +0900, Fujii Masao wrote
in
> On Thu, Jan 14, 2021 at 10:10 AM Kyotaro Horiguchi
> wrote:
> >
> > At Wed, 13 Jan 2021 16:51:32 -0300, Alvaro Herrera
> > wrote in
> > > On 2021-Jan-13, Fujii Masao wrote:
> > >
> >
/walsender.c:1233:WalSndPrepareWrite(LogicalDecodingContext
> *ctx, XLogRecPtr lsn, TransactionId xid, bool last_write)
> src/backend/replication/walsender.c:1255: * Actually write out data
> previously prepared by WalSndPrepareWrite out to
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 12 Jan 2021 18:58:08 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Fri, 08 Jan 2021 17:52:21 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > At Fri, 08 Jan 2021 14:47:05 +0900 (JST), Kyotaro Horiguchi
> > wrote in
> > > This version RelationChangePers
Hello.
The commit 4656e3d668 (debug_invalidate_system_caches_always)
conflicted with this patch. Rebased.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From ec069488fd2675369530f3f967f02a7b683f0a7f Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Wed, 18 Nov 2020 16
At Sat, 19 Dec 2020 17:55:22 +0900, Etsuro Fujita
wrote in
> On Mon, Dec 14, 2020 at 4:01 PM Kyotaro Horiguchi
> wrote:
> > At Sat, 12 Dec 2020 18:25:57 +0900, Etsuro Fujita
> > wrote in
> > > On Fri, Nov 20, 2020 at 3:51 PM Kyotaro Horiguchi
> > > wrote
; + errhint("You should turn off hot_standby here.")));
Since it's obvious that the change in a primary cannot be propagted by
taking a backup or starting replication, the first sentence reads to
me as "you should retake a base-backup from a primary where wal_level
is replica or higher". So *I* don't think it needs a fix.
Any thoughts?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
king a backup or starting replication, the first sentence reads to
> >me as "you should retake a base-backup from a primary where wal_level
> >is replica or higher". So *I* don't think it needs a fix.
> I think this HINT is want to guide users to finish this recovery, an
Thank you for the comments, Noah and Andres.
At Fri, 15 Jan 2021 20:38:16 -0800, Noah Misch wrote in
> On Wed, Jan 13, 2021 at 04:07:05PM +0900, Kyotaro Horiguchi wrote:
> > The definition of the macro RelationNeedsWAL has been changed by
> > c6b92041d3 to include conditions rel
ot;
> xact1: ALTER TABLE t SET TABLESPACE something; -- start skipping WAL
> xact1: SELECT count(*) FROM t; -- no error, wanted "snapshot too old"
>
> Is that plausible?
Thank you for the consideration and yes. But I get "snapshot too old"
from the last query wi
At Fri, 15 Jan 2021 08:56:27 +0100, Peter Eisentraut
wrote in
> On 2020-08-31 11:03, Kyotaro Horiguchi wrote:
> > At Tue, 18 Aug 2020 16:43:47 +0900 (JST), Kyotaro Horiguchi
> > wrote in
> >> Thank you very much. I'll do that after some polishing.
> &g
At Mon, 18 Jan 2021 17:30:22 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Sun, 17 Jan 2021 23:02:18 -0800, Noah Misch wrote in
> > On Sun, Jan 17, 2021 at 10:36:31PM -0800, Noah Misch wrote:
> > > I wrote the above based on the "PageGetLSN(page) > (snaps
users wait,
> I'm ok for the current interface. I don't feel the need of
> pg_is_wal_replay_paluse_requeseted().
FWIW, the name "pg_is_wal_replay_paused" is suggesting "to know
whether recovery is paused or not at present" and it would be
surprising to see it to wait for the recovery actually paused by
default.
I think there's no functions to wait for some situation at least for
now. If we wanted to wait for some condition to make, we would loop
over check-and-wait using plpgsql.
If you desire to wait to replication to pause by a function, I would
do that by adding a parameter to the function.
pg_is_wal_replay_paused(OPTIONAL bool wait_for_pause)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 19 Jan 2021 09:17:34 +0900 (JST), Kyotaro Horiguchi
wrote in
> By the way we can do the same thing on CA file/dir, but I personally
> think that the benefit from the specify-by-directory for CA files is
> far less than CRL files. So I'm not going to do this for CA files fo
At Tue, 19 Jan 2021 01:31:52 -0800, Noah Misch wrote in
> On Tue, Jan 19, 2021 at 01:48:31PM +0900, Kyotaro Horiguchi wrote:
> > I understand that you are suggesting that at least
> > TransactionIdLimitedForOldSnapshots should follow not only relation
> > persistence but Re
At Wed, 20 Jan 2021 17:34:44 +0900 (JST), Kyotaro Horiguchi
wrote in
> Anyway, it seems actually dangerous that cause pruning on wal-skipped
> relation.
>
> > with your patch versions. Could you try implementing both test procedures
> > in
> > src/test/modules/sn
gt;
> That seems reasonable to me. So +1.
That seems in the good balance. +1, too.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
by a single procedure. The last
function is equivalent to pg_last_wal_replay_lsn(). As the result, the
following procedure is provided in the attached.
pg_waitlsn(wait_lsn pg_lsn, timeout integer DEFAULT -1)
Any opinions mainly compared to implementation as a command?
regards.
--
Kyotaro Horiguchi
NTT O
At Thu, 21 Jan 2021 00:19:58 -0800, Noah Misch wrote in
> On Thu, Jan 21, 2021 at 12:28:44AM +0900, Kyotaro Horiguchi wrote:
> > At Wed, 20 Jan 2021 17:34:44 +0900 (JST), Kyotaro Horiguchi
> > wrote in
> > > Anyway, it seems actually dangerous that cause pruning on wa
ng to it one WAL record later than we
>* otherwise would is a minor issue, so it doesn't seem worth
>* adding another spinlock cycle to prevent that.
As the result, this patch tries to introduce several new checkpoints
to some delaying point so that that waits can find pause request in a
timely manner. I think we had better use locking (or atomics) for the
information instead of such scattered checkpoints if we expect that
machinery to work in a such syhcnronous manner.
That would make the tri-state state variable and many checkpoints
unnecessary. Maybe.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
es the
wrong result for nested portals.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Mon, 25 Jan 2021 10:05:19 +0530, Dilip Kumar wrote
in
> On Mon, Jan 25, 2021 at 8:42 AM Kyotaro Horiguchi
> wrote:
> >
> > At Sun, 24 Jan 2021 14:26:08 +0530, Dilip Kumar
> > wrote in
> > > On Sun, Jan 24, 2021 at 12:16 PM Bharath Rupireddy
> > &
Option))
I think we can use separate lstate state for each condition above
since IsPercentOption() gives a constant result through the execution
time. For example, LIMIT_PERCENT_TUPLESLOT_NOT_FILLED and
LIMIT_PERCENT_TUPLESLOT_FILLED and some derived states similar to the
non-percent path. I *fee
e the root cause but that way, even
> if the process didn’t delete it on destroying the parallel context, we
> can make sure to delete it on process exit.
>
> I think #1 is suitable for back branches. For HEAD, I think #2 and #3
> would be better in terms of not setting an implicit rule. Thoughts?
As far as we allow dms_detach being canceled, the problem persists
anywhat other we do. So #2 and #3 seems a bit too much. It seems to me
that #1 + omitting CHECK_FOR_INTERRUPTS() is suitable for all
branches.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
ts. ALTER
TABLE/INDEX ONLY doesn't, and the change takes effect on future
children.
We pursue relasing all fixes at once but we might release all fixes
other than some items that cannot be fixed for some technical reasons
at the time, like REPLICA IDENITTY.
I'm not sure how long we will wait for the time of release, though.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 26 Jan 2021 11:00:56 +0200, Heikki Linnakangas wrote
in
> On 26/01/2021 06:46, Kyotaro Horiguchi wrote:
> > Looking the comment of SharedFileSetOnDetach:
> > | * everything in them. We can't raise an error on failures, because this
> > | * runs
> > |
At Tue, 26 Jan 2021 11:43:21 +0200, Heikki Linnakangas wrote
in
> Hi,
>
> On 19/11/2020 07:25, Kyotaro Horiguchi wrote:
> > Performance measurement on the attached showed better result about
> > searching but maybe worse for cache entry creation. Each time number
>
At Thu, 14 Jan 2021 17:32:27 +0900 (JST), Kyotaro Horiguchi
wrote in
> The commit 4656e3d668 (debug_invalidate_system_caches_always)
> conflicted with this patch. Rebased.
At Wed, 27 Jan 2021 10:07:47 +0900 (JST), Kyotaro Horiguchi
wrote in
> (I found a bug in a benchmark-aid
At Tue, 26 Jan 2021 19:13:57 +, "Bossart, Nathan"
wrote in
> On 12/17/20, 9:15 PM, "Kyotaro Horiguchi" wrote:
> > At Thu, 17 Dec 2020 22:20:35 +, "Bossart, Nathan"
> > wrote in
> >> On 12/15/20, 2:33 AM, "Kyotaro Horiguchi&qu
At Wed, 27 Jan 2021 05:30:29 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > "CREATE TABLE" is not "CREATE LOGGED TABLE". We can assume that as
> > "CREATE TABLE", where the default logged-ness
> > va
the latter, we can rewrite the only use of it "if
(qkey->constr_queryno <= RI_PLAN_LAST_ON_PK)" not to use the macro.)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Wed, 27 Jan 2021 02:48:48 -0800, Noah Misch wrote in
> On Thu, Jan 21, 2021 at 01:23:36AM -0800, Noah Misch wrote:
> > On Thu, Jan 21, 2021 at 06:02:11PM +0900, Kyotaro Horiguchi wrote:
> > > Perhaps I'm missing something, but the patch doesn't pass the v5-00
At Wed, 27 Jan 2021 13:11:55 +0200, Heikki Linnakangas wrote
in
> On 27/01/2021 03:13, Kyotaro Horiguchi wrote:
> > At Thu, 14 Jan 2021 17:32:27 +0900 (JST), Kyotaro Horiguchi
> > wrote in
> >> The commit 4656e3d668 (debug_invalidate_system_caches_always)
> &g
At Thu, 28 Jan 2021 16:50:44 +0900 (JST), Kyotaro Horiguchi
wrote in
> I was going to write in the doc something like "you can inspect memory
> consumption by catalog caches using pg_backend_memory_contexts", but
> all the memory used by catalog cache is in CacheMemoryContext
At Wed, 27 Jan 2021 23:10:53 -0800, Noah Misch wrote in
> On Thu, Jan 28, 2021 at 12:06:27PM +0900, Kyotaro Horiguchi wrote:
> > At Wed, 27 Jan 2021 02:48:48 -0800, Noah Misch wrote in
> > > On Thu, Jan 21, 2021 at 01:23:36AM -0800, Noah Misch wrote:
> > > > On T
At Sat, 30 Jan 2021 22:20:19 +0100, Peter Eisentraut
wrote in
> On 2021-01-19 09:32, Kyotaro Horiguchi wrote:
> > At Tue, 19 Jan 2021 09:17:34 +0900 (JST), Kyotaro Horiguchi
> > wrote in
> >> By the way we can do the same thing on CA file/dir, but I personally
> >
r and it is predictable value by
> > the user. I don't have much objection to putting that check in the
> > recoveryApplyDelay as well but I feel it is not necessary. Any other
> > thoughts on this?
> >
> > > In addition, in RecoveryRequiresIntParameter, recovery should get paused
> > > if a parameter value has a problem. However,
> > > pg_get_wal_replay_pause_state
> > > will return 'pause requested' in this case. So, I think, we should pass
> > > RECOVERY_PAUSED to SetRecoveryPause() instead of RECOVERY_PAUSE_REQUESTED,
> > > or call CheckAndSetRecoveryPause() in the loop like recoveryPausesHere().
> >
> > Yeah, absolutely right, it must pass RECOVERY_PAUSED. I will change
> > this, thanks for noticing this.
>
> I have changed this in the new patch.
It seems to work well. The checkpoints seems to be placed properly.
+SetRecoveryPause(RecoveryPauseState state)
{
+ Assert(state >= RECOVERY_NOT_PAUSED && state <= RECOVERY_PAUSED);
I'm not sure that state worth FATAL. Isn't it enough to just ERROR
out like XLogFileRead?
CheckAndSetRecovery() has only one caller. I think it's better to
write the code directly.
I think the documentation of pg_wal_replay_pause needs to be a bit
more detailed about the difference between the two states "pause
requested" and "paused". Something like "A request doesn't mean that
recovery stops right away. If you want a guarantee that recovery is
actually paused, you need to check for the recovery pause state
returned by pg_wal_replay_pause_state(). Note that
pg_is_wal_repay_paused() returns whether a request is made."
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
"result" seems useless.
+ elog(LOG_SERVER_ONLY, "current backtrace:%s", errtrace.data);
+
+ errno = save_errno;
+}
You need to release the resouces held by the errtrace. And the
errtrace is a bit pointless. Why isn't it "backtrace"?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
is designed to *inhibit* timestamps from
being prepended. If that is actually intended, the symbol name should
be PQTRACE_NOOUTPUT_TIMESTAMP. Otherwise, the doc need to be fixed.
By the way removing "== 0" makes it difficult to tell whether the
condition is correct or not; I recommend to use '!= 0" rather than
removing '== 0'.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
ates to shared memory.
The type RecoveryState is int, which is of the native machine size
that is considered to be atomic as well as boolean. However, I don't
object to remove the phrase since that removal doesn't change the
point of the description.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
byte character
>
> > The fix is simple enough, so +1.
>
> Thanks, I'll commit and backpatch shortly.
I'm not sure the assumption in the second hunk always holds, but
that's fine at least with Shift-JIS and -2004 since they are two-byte
encoding.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Thu, 04 Feb 2021 10:16:47 +0900, torikoshia
wrote in
> Chengxi Sun, Yamada-san, Horiguchi-san,
>
> Thanks for all your comments.
> Adding only the number of generic plan execution seems acceptable.
>
> On Mon, Jan 25, 2021 at 2:10 PM Kyotaro Horiguchi
> wrote:
>
eing said, we might need a description about how we can specify
a unix socket directory in ecpg-connect.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 6b0a3067e6..f45892304d 100644
--
;\
-" WHERE substring(name,1,%d)='%s'"
+" WHERE substring(name,1,%1$d)='%2$s' "\
+"OR pg_catalog.lower(substring(name,1,%1$d))=pg_catalog.lower('%2$s')"
#define Query_for_list_of_show_vars \
"SELECT name FROM "\
=# set AP[tab]
=# set application_name _
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
low change the state from PAUSE to
REQUESTED via NOT_PAUSED between two successive loop condition checks?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Mon, 8 Feb 2021 17:05:52 +0530, Dilip Kumar wrote in
> On Mon, Feb 8, 2021 at 2:19 PM Yugo NAGATA wrote:
> >
> > On Mon, 08 Feb 2021 17:32:46 +0900 (JST)
> > Kyotaro Horiguchi wrote:
> >
> > > At Mon, 8 Feb 2021 14:12:35 +0900, Yugo NAGATA
> >
directory
+ for Unix-domain communications, an option host=
+ and single-quoted string must be used.
+ The notation rule is almost the same as libpq's one,
+ but the IPv6 address cannot be used here.
+
is not the tag to use for "host". is that.
If we change the "h
At Tue, 09 Feb 2021 13:58:14 +0900 (JST), Kyotaro Horiguchi
wrote in
> > I didn't care about the windows environment.
> > Somewhat WIN32 directive can be used for switching code, but I agree your
> > claims.
>
> This thread looks like discussing about unix-doma
At Tue, 9 Feb 2021 12:23:23 +0900, Yugo NAGATA wrote in
> On Tue, 09 Feb 2021 10:58:04 +0900 (JST)
> Kyotaro Horiguchi wrote:
> > I didn't asked about the internal logical correctness, but asked about
> > *actual harm* revealed to users. I don't see any actual harm i
At Tue, 9 Feb 2021 09:47:58 +0530, Dilip Kumar wrote in
> On Tue, Feb 9, 2021 at 8:54 AM Yugo NAGATA wrote:
> >
> > On Tue, 09 Feb 2021 10:58:04 +0900 (JST)
> > Kyotaro Horiguchi wrote:
> >
> > > At Mon, 8 Feb 2021 17:05:52 +0530, Dilip Kumar
> > &
At Tue, 9 Feb 2021 09:58:30 +0530, Bharath Rupireddy
wrote in
> On Tue, Feb 9, 2021 at 9:48 AM Dilip Kumar wrote:
> >
> > On Tue, Feb 9, 2021 at 8:54 AM Yugo NAGATA wrote:
> > >
> > > On Tue, 09 Feb 2021 10:58:04 +0900 (JST)
> > > Kyotaro Horiguchi w
Sorry, I made a mistake here.
At Tue, 09 Feb 2021 14:55:23 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Tue, 9 Feb 2021 09:47:58 +0530, Dilip Kumar wrote
> in
> > APIs the wait logic can be implemented in the application code which
> > is actually using these APIs and I
the initial size, we reuse it next
> time,
> + * because it would be allocated same size and the size is not
> big.
> + */
> + conn->fe_msg->max_fields != DEF_FE_MSGFIELDS)
>
> I'm not completely sure if other places interpose a block comment like this
> between if/for/while conditions, but I think it's better to put the comment
> before if.
(s/is/are/)
Agreed. At least it doesn't look good.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
s message to the log
This logs a byte sequence in hexadecimals. I would name that as
pqTraceEmitBytesLog(). Or pqTraceEmit(Be|Fe)BytesLog().
...I finish once here.
Is there any thoughts? Optinions on the namings?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Tue, 9 Feb 2021 08:10:19 +, "tsunakawa.ta...@fujitsu.com"
wrote in
> From: Kyotaro Horiguchi
> > > (45)
> > This looks like a fusion of PQtrace and PQtraceEX. By the way, the
> > timestamp flag is needed at log emittion. So we can change the state
>
the existing API and return type, a new function
> pg_get_wal_replay_pause_state is introduced.
I mentioned about IN parameters, not OUTs. IN parameters can be
optional to accept existing usage. pg_wal_replay_pause() is changed
that way in the attached.
If all of you still disagree with my proposal, I wi
<-- CID 1446240 (#1 of 1):
> Out-of-bounds access (OVERRUN)
> pg_cryptohash_free(ctx);
> return
> }
>
> Attached has a patch with suggestions to make things better.
I'm not sure about the details, but it looks like broken.
make complains for inconsistent prototypes abd cryptohahs.
At Wed, 10 Feb 2021 12:13:44 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Tue, 9 Feb 2021 22:01:45 -0300, Ranier Vilela wrote
> in
> > Hi Hackers,
> >
> > Per Coverity.
> >
> > Coverity complaints about pg_cryptohash_final function.
> > And I agree
g_cryptohash_final to take the buffer length, it should
error-out or assert-out if the length is too small rather than copy a
part of the digest bytes. (In short, it would only be assertion-use.)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
as an intrinsic aggregation?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
opose a change of the doc as attached.
What do you think about this?
[1]
https://www.postgresql.org/message-id/20211202.134619.1052008069537649171.horikyota.ntt%40gmail.com
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/con
At Thu, 02 Dec 2021 13:54:41 +0900 (JST), Kyotaro Horiguchi
wrote in
> As discussed in the thread [1], I find the wording "SSL server
> certificate revocation list" as misleading or plain wrong.
FWIW, I'm convinced that that's plain wrong after finding some
o
ing and comitting this.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Fri, 3 Dec 2021 15:16:55 +0900, Michael Paquier wrote
in
> On Fri, Sep 17, 2021 at 02:45:57AM +0900, Kyotaro Horiguchi wrote:
> > This test fails for the same reason, but after fixing it the result
> > contains \a (BEL) in the output on my CentOS8. I'm not sure what i
re necessary. Could you clarify that?
It probably comes from my request, just for safety for uncertain
future. They are actually not needed under the current usage of the
function, so *I* would not object to removing them if you are strongly
feel them out of place. In that case, since NULL's leads to SEGV
outright, we would even not need assertions instead.
> + Same as , this is a
> + printf-style string. Accepted escapes are
> + bit different from ,
> + but padding can be used like as it.
>
> This description needs to be updated.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
server would start with a clean startup next time. We could treat
DB_IN_END_OF_RECOVERY_CHECKPOINT as safe state to skip recovery but I
don't think we need to preserve that behavior.
In other places, server log and ps display specifically, we already
make distinction between end-of-recove
At Tue, 09 Nov 2021 16:27:51 +0900 (JST), Kyotaro Horiguchi
wrote in
> This is the updated version.
>
> - emode_for_currupt_record() now uses currentSource instead of
> readSource.
>
> - If zero record length is faced, make sure the whole header is zeroed
> before d
;
> > Of course, I could be off-base and others might agree that this new
> > state would be nice to have.
>
> Let's see what others have to say about this.
I see it a bit too complex for the advantage. When end-of-recovery
checkpoint takes so long, that state is shown in server log, which
operators would look into before the control file.
> [1] -
> https://www.postgresql.org/message-id/CALj2ACVn5M8xgQ3RD%3D6rSTbbXRBdBWZ%3DTTOBOY_5%2BedMCkWjHA%40mail.gmail.com
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
s well.
I'd prefer "during before_shmem_exit()" than "in
ReplicationSlotBeforeShmemExit() callback" here. (But the current
wording is also fine by me.)
The attached detects that bug, but I'm not sure it's worth expending
test time, or this might be in the server
614 to release replication slot "s1" because it's
> restart_lsn 0/2CA0 exceeds max_slot_wal_keep_size.
What do you think about this?
[1]
https://www.postgresql.org/message-id/20211214.101137.379073733372253470.horikyota.ntt%40gmail.com
--
Kyotaro Horiguchi
NTT Open Source
errdetail("The process with PID %d released
it.", pid)));
+
This is wrong. I see a "become inactive" message if I droped an
"inactive" replication slot. The reason the inactive slot looks as if
it were acquired is it is temporarily aquired as a
At Tue, 14 Dec 2021 19:31:21 +0530, Ashutosh Bapat
wrote in
> On Tue, Dec 14, 2021 at 9:35 AM Kyotaro Horiguchi
> wrote:
> > > [17605] LOG: terminating process 17614 to release replication slot "s1"
> > + [17605] DETAIL: The slot's restart_lsn 0/2CA0
At Wed, 15 Dec 2021 16:20:55 +0900 (JST), Kyotaro Horiguchi
wrote in
> adjusted so that it treats null as false. On the way doing this, the
> bug #17334 [2] and another bug raised earlier [3] are naturally fixed.
That being said, even if this patch were committed to the master
branch, we
rchr(host, '/')
+ && pg_inet_net_pton(PGSQL_AF_INET6, host, addr, -1) ==
128)
If a cidr is given, pg_inet_net_pton returns a number less than 128 so
we don't need to check '/' explicity? (I'm not sure '/128' is
sensible but doesn't harm..)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
e next segment of
the last WAL in archive but that would invite more complex problems.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
function
itself changes its mind to set values[i] to null while we ignore the
possibility that some module at remote is modified so that some global
variables to be NULL. I don't mind wheter we care for NULLs or not
but I think we should be consistent at least in a single patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
mbers are sent to stats collector by pgstat_report_stat.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Thu, 16 Dec 2021 18:44:54 +, Jacob Champion wrote
in
> On Thu, 2021-12-16 at 14:54 +0900, Kyotaro Horiguchi wrote:
> > It seems like saying that we must search for iPAddress and mustn't use
> > CN nor dNSName if the client connected using IP address. Otherwise, if
&g
; #2. add the check but not use "[unknown]"
> #3. don't add the check (i.e., what the current patch does)
>
> For now, I'm ok to choose #2 or #3.
As I said before, given that we don't show "unkown" or somethig like
as the fallback, I'm fine with not having a NULL check since anyway it
bumps into SEGV immediately. In short I'm fine with #3 here.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Sorry for the silly mistake.
At Fri, 17 Dec 2021 15:40:10 +0900 (JST), Kyotaro Horiguchi
wrote in
> > NSS departs slightly from the spec and will additionally try to match
> > an IP address against the CN, but only if there are no iPAddresses in
> > the SAN. It roughly mat
ionCreate/DropInitFork's behavior for non-target
relations. (From Jakub's work).
- Fixed wording of some comments.
- As revisited, I found a bug around recovery. If the logged-ness of a
relation gets flipped repeatedly in a transaction, duplicate
pending-delete entries are accumula
301 - 400 of 3221 matches
Mail list logo
