Hi,
On 2020-06-16 19:46:29 -0400, Tom Lane wrote:
> Andres Freund writes:
> > I experimented with making the compiler warn about about some of these
> > kinds of mistakes without needing full test coverage:
>
> > I was able to get clang to warn about things like using palloc in signal
> > handler
Andres Freund writes:
> I experimented with making the compiler warn about about some of these
> kinds of mistakes without needing full test coverage:
> I was able to get clang to warn about things like using palloc in signal
> handlers, or using palloc while holding a spinlock. Which would be
>
Hi,
On 2020-06-03 00:36:34 -0400, Tom Lane wrote:
> Should we think about adding automated detection of this type of
> mistake? I don't like the attached as-is because of the #include
> footprint expansion, but maybe we can find a better way.
I experimented with making the compiler warn about ab
On Tue, Jun 9, 2020 at 8:12 PM Andres Freund wrote:
> I don't think the size is worth of concern in this case, and I'm not
> sure there's any current case where it's really worth spending effort
> reducing size. But if there is: It seems possible to reduce the size.
Yeah, I don't think it's very
Hi,
On 2020-06-09 15:20:08 -0400, Robert Haas wrote:
> If you're worried about space, an LWLock is only 16 bytes, and the
> slock_t that we'd be replacing is currently at the end of the struct
> so presumably followed by some padding.
I don't think the size is worth of concern in this case, and I
Hi,
On 2020-06-09 19:24:15 -0400, Tom Lane wrote:
> Robert Haas writes:
> > On Tue, Jun 9, 2020 at 1:59 PM Tom Lane wrote:
> >> When I went through the existing spinlock stanzas, the only thing that
> >> really made me acutely uncomfortable was the chunk in pg_stat_statement's
> >> pgss_store(),
Robert Haas writes:
> On Tue, Jun 9, 2020 at 1:59 PM Tom Lane wrote:
>> When I went through the existing spinlock stanzas, the only thing that
>> really made me acutely uncomfortable was the chunk in pg_stat_statement's
>> pgss_store(), lines 1386..1438 in HEAD.
> I mean, what would be wrong wit
On Tue, Jun 9, 2020 at 1:59 PM Tom Lane wrote:
> Robert Haas writes:
> > Removing some of these spinlocks and replacing them with LWLocks might
> > also be worth considering.
>
> When I went through the existing spinlock stanzas, the only thing that
> really made me acutely uncomfortable was the
Robert Haas writes:
> Removing some of these spinlocks and replacing them with LWLocks might
> also be worth considering.
When I went through the existing spinlock stanzas, the only thing that
really made me acutely uncomfortable was the chunk in pg_stat_statement's
pgss_store(), lines 1386..1438
On Wed, Jun 3, 2020 at 12:36 AM Tom Lane wrote:
> Should we think about adding automated detection of this type of
> mistake? I don't like the attached as-is because of the #include
> footprint expansion, but maybe we can find a better way.
I think it would be an excellent idea.
Removing some o
Michael Paquier writes:
> On Wed, Jun 03, 2020 at 12:36:34AM -0400, Tom Lane wrote:
>> Should we think about adding automated detection of this type of
>> mistake? I don't like the attached as-is because of the #include
>> footprint expansion, but maybe we can find a better way.
> I think that t
On Wed, Jun 03, 2020 at 12:36:34AM -0400, Tom Lane wrote:
> Should we think about adding automated detection of this type of
> mistake? I don't like the attached as-is because of the #include
> footprint expansion, but maybe we can find a better way.
I think that this one first boils down to the
Kyotaro Horiguchi writes:
> I looked through 224 locations where SpinLockAcquire and found some.
Yeah, I made a similar scan and arrived at about the same conclusions.
I think that the memcpy and strlcpy calls are fine; at least, we've got
to transport data somehow and it's not apparent why those
At Wed, 03 Jun 2020 02:00:53 -0400, Tom Lane wrote in
> ... and InvalidateObsoleteReplicationSlots(), too.
>
> I am detecting a pattern here.
I looked through 224 locations where SpinLockAcquire and found some.
LogicalIncreaseRestartDecodingForSlot is spotted by Michael.
pg_get_replication_slo
... and InvalidateObsoleteReplicationSlots(), too.
I am detecting a pattern here.
regards, tom lane
Michael Paquier writes:
> On Wed, Jun 03, 2020 at 01:27:51AM -0400, Tom Lane wrote:
>> I'm inclined to think that memcpy'ing the ReplicationSlot struct
>> into a local variable might be the best way, replacing all the
>> piecemeal copying these stanzas are doing right now.
> +1. And I guess that
On Wed, Jun 03, 2020 at 01:27:51AM -0400, Tom Lane wrote:
> I'm inclined to think that memcpy'ing the ReplicationSlot struct
> into a local variable might be the best way, replacing all the
> piecemeal copying these stanzas are doing right now. memcpy() of
> a fixed amount of data isn't quite stra
I wrote:
> Ugh, that is just horrid. I experimented with the attached patch
> but it did not find any other problems.
It occurred to me to add NotHoldingSpinLock() into palloc and
friends, and look what I found in copy_replication_slot:
SpinLockAcquire(&s->mutex);
src_isl
On Wed, Jun 03, 2020 at 12:36:34AM -0400, Tom Lane wrote:
> Ugh, that is just horrid. I experimented with the attached patch
> but it did not find any other problems.
Oh. I can see the same "ifndef FRONTEND" logic all around the place
as I did on my local branch :)
> Still, that only proves som
On Wed, Jun 03, 2020 at 08:52:08AM +0530, Amit Kapila wrote:
> Do you mean to say correct?
Nope, I really meant that the code before caa3c42 is incorrect, and I
am glad that it got fixed. Sorry if that sounded confusing.
--
Michael
signature.asc
Description: PGP signature
Michael Paquier writes:
> Indeed, this was incorrect. And you may not have noticed, but we have
> a second instance of that in LogicalIncreaseRestartDecodingForSlot()
> that goes down to 9.4 and b89e151. I used a dirty-still-efficient
> hack to detect that, and that's the only instance I have sp
On Wed, Jun 3, 2020 at 8:35 AM Michael Paquier wrote:
>
> On Wed, Jun 03, 2020 at 09:18:19AM +0900, Kyotaro Horiguchi wrote:
> > Thanks to all!
>
> Indeed, this was incorrect.
>
Do you mean to say correct?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jun 03, 2020 at 09:18:19AM +0900, Kyotaro Horiguchi wrote:
> Thanks to all!
Indeed, this was incorrect. And you may not have noticed, but we have
a second instance of that in LogicalIncreaseRestartDecodingForSlot()
that goes down to 9.4 and b89e151. I used a dirty-still-efficient
hack to
At Tue, 2 Jun 2020 19:24:16 +0900, Fujii Masao
wrote in
> Thanks! I pushed the patch.
Thanks to all!
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2020/06/02 17:42, Amit Kapila wrote:
On Tue, Jun 2, 2020 at 2:05 PM Fujii Masao wrote:
On 2020/06/02 16:15, Kyotaro Horiguchi wrote:
Hello.
I noticed that UpdateSpillStats calls "elog(DEBUG2" within
SpinLockAcquire section on MyWalSnd. The lock doesn't protect rb and
in the first plac
On Tue, Jun 2, 2020 at 2:05 PM Fujii Masao wrote:
>
> On 2020/06/02 16:15, Kyotaro Horiguchi wrote:
> > Hello.
> >
> > I noticed that UpdateSpillStats calls "elog(DEBUG2" within
> > SpinLockAcquire section on MyWalSnd. The lock doesn't protect rb and
> > in the first place the rb cannot be modifi
On 2020/06/02 16:15, Kyotaro Horiguchi wrote:
Hello.
I noticed that UpdateSpillStats calls "elog(DEBUG2" within
SpinLockAcquire section on MyWalSnd. The lock doesn't protect rb and
in the first place the rb cannot be modified during the function is
running.
It should be out of the lock sect
Hello.
I noticed that UpdateSpillStats calls "elog(DEBUG2" within
SpinLockAcquire section on MyWalSnd. The lock doesn't protect rb and
in the first place the rb cannot be modified during the function is
running.
It should be out of the lock section.
regards.
--
Kyotaro Horiguchi
NTT Open Sour
28 matches
Mail list logo
