Re: Online enabling of checksums

2019-01-31 Thread Magnus Hagander
On Thu, Jan 31, 2019 at 11:57 AM Andres Freund wrote: > On 2018-09-30 10:48:36 +0200, Tomas Vondra wrote: > > > > > > On 09/29/2018 06:51 PM, Stephen Frost wrote: > > > Greetings, > > > > > > * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: > > >> On 09/29/2018 02:19 PM, Stephen Frost wrote:

Re: Online enabling of checksums

2019-01-31 Thread Andres Freund
On 2018-09-30 10:48:36 +0200, Tomas Vondra wrote: > > > On 09/29/2018 06:51 PM, Stephen Frost wrote: > > Greetings, > > > > * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: > >> On 09/29/2018 02:19 PM, Stephen Frost wrote: > >>> * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: >

Re: Online enabling of checksums

2018-09-30 Thread Tomas Vondra
On 09/29/2018 06:51 PM, Stephen Frost wrote: > Greetings, > > * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: >> On 09/29/2018 02:19 PM, Stephen Frost wrote: >>> * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: While looking at the online checksum verification patch (which I

Re: Online enabling of checksums

2018-09-29 Thread Stephen Frost
Greetings, * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: > On 09/29/2018 02:19 PM, Stephen Frost wrote: > > * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: > >> While looking at the online checksum verification patch (which I guess > >> will get committed before this one), it occurred

Re: Online enabling of checksums

2018-09-29 Thread Tomas Vondra
On 09/29/2018 02:19 PM, Stephen Frost wrote: > Greetings, > > * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: >> While looking at the online checksum verification patch (which I guess >> will get committed before this one), it occurred to me that disabling >> checksums may need to be more

Re: Online enabling of checksums

2018-09-29 Thread Stephen Frost
Greetings, * Tomas Vondra (tomas.von...@2ndquadrant.com) wrote: > While looking at the online checksum verification patch (which I guess > will get committed before this one), it occurred to me that disabling > checksums may need to be more elaborate, to protect against someone > using the stale

Re: Online enabling of checksums

2018-09-29 Thread Tomas Vondra
Hi, While looking at the online checksum verification patch (which I guess will get committed before this one), it occurred to me that disabling checksums may need to be more elaborate, to protect against someone using the stale flag value (instead of simply switching to "off" assuming that's

Re: Online enabling of checksums

2018-08-01 Thread Sergei Kornilov
Hi > This doesn't test the consequences of the restart being skipped, nor > does it review on a code level the correctness. I check not only one stuff during review. I look code too: bgworker checksumhelper.c registered with: > bgw.bgw_start_time = BgWorkerStart_RecoveryFinished; And then

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 21:03:22 +0300, Sergei Kornilov wrote: > > They fail over to a secondary to do maintenance on a primary. > But this is not problem even in current patch state. We can restart replica > before failover and it works. I tested this behavior during my review. > We can: > - call

Re: Online enabling of checksums

2018-08-01 Thread Sergei Kornilov
Hi > They fail over to a secondary to do maintenance on a primary. But this is not problem even in current patch state. We can restart replica before failover and it works. I tested this behavior during my review. We can: - call pg_enable_data_checksums() on master - wait change data_checksums

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 10:34:55 -0700, Joshua D. Drake wrote: > Lastly, I think Alvaro has a point with the incremental development and I > also think some others on this thread need to, "show me the patch" instead > of being armchair directors of development. Oh, FFS. I pointed out the issue that

Re: Online enabling of checksums

2018-08-01 Thread Joshua D. Drake
On 08/01/2018 09:20 AM, Alvaro Herrera wrote: my problem is that I think the "restart" approach is just using the entirely wrong hammer to solve the problem at hand. At the very least it's very problematic in respect to replicas, which need to know about the setting too, and can have similar

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 12:45:27 -0400, Bruce Momjian wrote: > On Wed, Aug 1, 2018 at 09:39:43AM -0700, Andres Freund wrote: > > On 2018-08-01 12:36:13 -0400, Bruce Momjian wrote: > > > This patchset is weird because it is perhaps our first case of trying to > > > change the state of the server while

Re: Online enabling of checksums

2018-08-01 Thread Bruce Momjian
On Wed, Aug 1, 2018 at 09:39:43AM -0700, Andres Freund wrote: > On 2018-08-01 12:36:13 -0400, Bruce Momjian wrote: > > This patchset is weird because it is perhaps our first case of trying to > > change the state of the server while it is running. We just don't have > > an established protocol

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
On 2018-08-01 12:36:13 -0400, Bruce Momjian wrote: > This patchset is weird because it is perhaps our first case of trying to > change the state of the server while it is running. We just don't have > an established protocol for how to orchestrate that, so we are limping > along toward a

Re: Online enabling of checksums

2018-08-01 Thread Bruce Momjian
On Tue, Jul 31, 2018 at 04:05:23PM -0700, Andres Freund wrote: > Hi, > > On 2018-07-31 18:56:29 -0400, Alvaro Herrera wrote: > > In the spirit of supporting incremental development, I think it's quite > > sensible to get the current thing done, then see what it takes to get > > the next thing

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 18:25:48 +0200, Tomas Vondra wrote: > Sure, if there are issues with this approach, that would make it > unacceptable. I'm not sure why would it be an issue for replicas (which is > what you mention elsewhere), considering those don't write data and so can't > fail to update a

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
On 2018-08-01 12:20:12 -0400, Alvaro Herrera wrote: > Hello > > On 2018-Aug-01, Andres Freund wrote: > > > My problem isn't just that I shouldn't think this should be committed > > without at least a firm committement to do better, > > I take "I think this shouldn't be committed" is what you

Re: Online enabling of checksums

2018-08-01 Thread Alvaro Herrera
Hello On 2018-Aug-01, Andres Freund wrote: > My problem isn't just that I shouldn't think this should be committed > without at least a firm committement to do better, I take "I think this shouldn't be committed" is what you meant. I'm not sure I agree with this line of argument. The reality

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 11:15:38 +0200, Tomas Vondra wrote: > On 08/01/2018 10:40 AM, Michael Banck wrote: > > If this was one week before feature freeze, I would agree with you > > that it makes sense to ship it with the restart requirement rather > > than not shipping it at all. But we're several

Re: Online enabling of checksums

2018-08-01 Thread Andres Freund
Hi, On 2018-08-01 10:40:24 +0200, Michael Banck wrote: > If this was one week before feature freeze, I would agree with you that > it makes sense to ship it with the restart requirement rather than not > shipping it at all. But we're several commitfests away from v12, so > making an effort to

Re: Online enabling of checksums

2018-08-01 Thread Sergei Kornilov
Hello I think one restart is acceptable for such feature. I doubt user want often disable-enable checksums. In many cases checksums will be enabled one time during all cluster life. We need more downtimes for minor updates (4/year) and changes in config PGC_POSTMASTER (max_connections or

Re: Online enabling of checksums

2018-08-01 Thread Tomas Vondra
On 08/01/2018 10:40 AM, Michael Banck wrote: > Hi, > > Am Dienstag, den 31.07.2018, 18:56 -0400 schrieb Alvaro Herrera: >> The ability to get checksums enabled is a killer feature; the >> ability to do it with no restart ... okay, it's better than >> requiring a restart, but it's not *that* big a

Re: Online enabling of checksums

2018-08-01 Thread Michael Banck
Hi, Am Dienstag, den 31.07.2018, 18:56 -0400 schrieb Alvaro Herrera: > The ability to get checksums enabled is a killer feature; the ability to > do it with no restart ... okay, it's better than requiring a restart, > but it's not *that* big a deal. Well, it's a downtime and service interruption

Re: Online enabling of checksums

2018-07-31 Thread Andres Freund
Hi, On 2018-07-31 18:56:29 -0400, Alvaro Herrera wrote: > In the spirit of supporting incremental development, I think it's quite > sensible to get the current thing done, then see what it takes to get > the next thing done. Each is an improvement on its own merits. And it > doesn't have to be

Re: Online enabling of checksums

2018-07-31 Thread Alvaro Herrera
On 2018-Jul-31, Tom Lane wrote: > Andres Freund writes: > > On 2018-07-31 23:20:27 +0200, Daniel Gustafsson wrote: > >> Not really arguing for or against, but just to understand the reasoning > >> before > >> starting hacking. Why do we feel that a restart (intended for safety > >> here) in >

Re: Online enabling of checksums

2018-07-31 Thread Tom Lane
Andres Freund writes: > On 2018-07-31 23:20:27 +0200, Daniel Gustafsson wrote: >> Not really arguing for or against, but just to understand the reasoning >> before >> starting hacking. Why do we feel that a restart (intended for safety here) >> in >> this case is a burden on a use-once

Re: Online enabling of checksums

2018-07-31 Thread Andres Freund
Hi, On 2018-07-31 23:20:27 +0200, Daniel Gustafsson wrote: > > On 26 Jul 2018, at 19:35, Andres Freund wrote: > > On July 26, 2018 10:03:39 AM PDT, Robert Haas > > wrote: > > >> Why can't we do better? > > > > I don't think it's that hard to do better. IIRC I

Re: Online enabling of checksums

2018-07-31 Thread Daniel Gustafsson
> On 31 Jul 2018, at 21:52, Joshua D. Drake wrote: > > On 07/31/2018 12:45 PM, Bruce Momjian wrote: >>> Thanks for reviewing, I’ve updated the patch with the above mentioned >>> incorrect >>> linkends as well as fixed the comments you made in a previous review. >>> >>> The CF-builder-bot is

Re: Online enabling of checksums

2018-07-31 Thread Bruce Momjian
On Tue, Jul 31, 2018 at 12:52:40PM -0700, Joshua Drake wrote: > On 07/31/2018 12:45 PM, Bruce Momjian wrote: > > > >>Hi!, > >> > >>Thanks for reviewing, I’ve updated the patch with the above mentioned > >>incorrect > >>linkends as well as fixed the comments you made in a previous review. > >> >

Re: Online enabling of checksums

2018-07-31 Thread Joshua D. Drake
On 07/31/2018 12:45 PM, Bruce Momjian wrote: Hi!, Thanks for reviewing, I’ve updated the patch with the above mentioned incorrect linkends as well as fixed the comments you made in a previous review. The CF-builder-bot is red, but it’s because it’s trying to apply the already committed patch

Re: Online enabling of checksums

2018-07-31 Thread Bruce Momjian
On Wed, Jul 25, 2018 at 11:35:31AM +0200, Daniel Gustafsson wrote: > > On 24 Jul 2018, at 11:05, Sergei Kornilov wrote: > > > > The following review has been posted through the commitfest application: > > make installcheck-world: tested, failed > > Implements feature: not tested > > Spec

Re: Online enabling of checksums

2018-07-26 Thread Andres Freund
On July 26, 2018 10:03:39 AM PDT, Robert Haas wrote: >On Tue, Jun 26, 2018 at 7:45 AM, Magnus Hagander >wrote: >> PFA an updated version of the patch for the next CF. We believe this >one >> takes care of all the things pointed out so far. >> >> For this version, we "implemented" the

Re: Online enabling of checksums

2018-07-26 Thread Robert Haas
On Tue, Jun 26, 2018 at 7:45 AM, Magnus Hagander wrote: > PFA an updated version of the patch for the next CF. We believe this one > takes care of all the things pointed out so far. > > For this version, we "implemented" the MegaExpensiveRareMemoryBarrier() by > simply requiring a restart of

Re: Online enabling of checksums

2018-07-25 Thread Sergei Kornilov
Hello Thank you for update! I did only quick test now: patch applied and build clean. But i have reproducible error during check-world: > t/001_standby_checksum.pl .. 6/10 > # Failed test 'ensure checksums are enabled on standby' > # at t/001_standby_checksum.pl line 84. > # got:

Re: Online enabling of checksums

2018-07-25 Thread Daniel Gustafsson
> On 24 Jul 2018, at 11:05, Sergei Kornilov wrote: > > The following review has been posted through the commitfest application: > make installcheck-world: tested, failed > Implements feature: not tested > Spec compliant: not tested > Documentation:tested, failed > >

Re: Online enabling of checksums

2018-07-24 Thread Sergei Kornilov
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: not tested Spec compliant: not tested Documentation:tested, failed Hello As i wrote few weeks ago i can not build documentation due

Re: Online enabling of checksums

2018-06-26 Thread Sergei Kornilov
Hello I tried build this patch and got error during make docs > postgres.sgml:19626: element xref: validity error : IDREF attribute linkend > references an unknown ID "runtime-checksumhelper-cost-limit" > postgres.sgml:19625: element xref: validity error : IDREF attribute linkend > references

Re: Online enabling of checksums

2018-06-26 Thread Magnus Hagander
On Mon, Apr 9, 2018 at 7:22 PM, Magnus Hagander wrote: > On Sat, Apr 7, 2018 at 6:22 PM, Andres Freund wrote: > >> Hi, >> >> On 2018-04-07 08:57:03 +0200, Magnus Hagander wrote: >> > Note however that I'm sans-laptop until Sunday, so I will revert it >> then or >> > possibly Monday. >> >> I'll

Re: Online enabling of checksums

2018-04-15 Thread Magnus Hagander
On Tue, Apr 10, 2018 at 6:18 PM, Robert Haas wrote: > On Fri, Apr 6, 2018 at 8:59 PM, Andres Freund wrote: > > This is PROPARALLEL_RESTRICTED. That doesn't strike me right, shouldn't > > they be PROPARALLEL_UNSAFE? It might be fine, but I'd not want to

Re: Online enabling of checksums

2018-04-10 Thread Robert Haas
On Fri, Apr 6, 2018 at 8:59 PM, Andres Freund wrote: > This is PROPARALLEL_RESTRICTED. That doesn't strike me right, shouldn't > they be PROPARALLEL_UNSAFE? It might be fine, but I'd not want to rely > on it. Just a fine-grained note on this particular point: It's totally

Re: Online enabling of checksums

2018-04-09 Thread Magnus Hagander
On Sat, Apr 7, 2018 at 6:22 PM, Andres Freund wrote: > Hi, > > On 2018-04-07 08:57:03 +0200, Magnus Hagander wrote: > > Note however that I'm sans-laptop until Sunday, so I will revert it then > or > > possibly Monday. > > I'll deactive the isolationtester tests until then.

Re: Online enabling of checksums

2018-04-07 Thread Andres Freund
Hi, On 2018-04-07 08:57:03 +0200, Magnus Hagander wrote: > Note however that I'm sans-laptop until Sunday, so I will revert it then or > possibly Monday. I'll deactive the isolationtester tests until then. They've been intermittently broken for days now and prevent other tests from being

Re: Online enabling of checksums

2018-04-07 Thread Andres Freund
Hi, On 2018-04-07 10:14:49 +0200, Michael Banck wrote: > Can the pg_verify_checksums command be kept at least, please? > > AFAICT this one is not contentious, the code is isolated, it's really > useful, orthogonal to online checksum activation and argueably could've > been committed as a

Re: Online enabling of checksums

2018-04-07 Thread Michael Banck
Hi, On Sat, Apr 07, 2018 at 08:57:03AM +0200, Magnus Hagander wrote: > On Sat, Apr 7, 2018 at 6:26 AM, Andres Freund wrote: > > If it's not obvious: This isn't ready, should be reverted, cleaned up, > > and re-submitted for v12. > > While I do think that it's still

Re: Online enabling of checksums

2018-04-07 Thread Magnus Hagander
On Sat, Apr 7, 2018 at 6:26 AM, Andres Freund wrote: > On 2018-04-06 17:59:28 -0700, Andres Freund wrote: > > + /* > > + * Create a database list. We don't need to concern ourselves with > > + * rebuilding this list during runtime since any database created >

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
Here's a pass through the patch: @@ -1033,7 +1034,7 @@ XLogInsertRecord(XLogRecData *rdata, Assert(RedoRecPtr < Insert->RedoRecPtr); RedoRecPtr = Insert->RedoRecPtr; } - doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites); +

Re: Online enabling of checksums

2018-04-06 Thread Stephen Frost
Andres, * Andres Freund (and...@anarazel.de) wrote: > On 2018-04-06 19:31:56 -0400, Stephen Frost wrote: > > I'm quite sure that bringing up MERGE in this thread and saying it needs > > to be reverted without even having the committer of that feature on the > > CC list isn't terribly useful and

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-07 01:27:13 +0200, Daniel Gustafsson wrote: > > On 07 Apr 2018, at 01:13, Andres Freund wrote: > > > > On 2018-04-07 01:04:50 +0200, Daniel Gustafsson wrote: > >>> I'm fairly certain that the bug here is a simple race condition in the > >>> test (not the main

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 19:31:56 -0400, Stephen Frost wrote: > Greetings, > > * Robert Haas (robertmh...@gmail.com) wrote: > > On Fri, Apr 6, 2018 at 6:56 PM, Andres Freund wrote: > > > no one can entirely quibble with the rationale that this is ok (I'll > > > post a patch cleaning up

Re: Online enabling of checksums

2018-04-06 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Apr 6, 2018 at 6:56 PM, Andres Freund wrote: > > no one can entirely quibble with the rationale that this is ok (I'll > > post a patch cleaning up the atomics simulation of flags in a bit), but > > this is

Re: Online enabling of checksums

2018-04-06 Thread Daniel Gustafsson
> On 07 Apr 2018, at 01:13, Andres Freund wrote: > > On 2018-04-07 01:04:50 +0200, Daniel Gustafsson wrote: >>> I'm fairly certain that the bug here is a simple race condition in the >>> test (not the main code!): >> >> I wonder if it may perhaps be a case of both? > > See

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-07 01:04:50 +0200, Daniel Gustafsson wrote: > > I'm fairly certain that the bug here is a simple race condition in the > > test (not the main code!): > > I wonder if it may perhaps be a case of both? See my other message about the atomic fallback bit. > > It's > > exceedingly

Re: Online enabling of checksums

2018-04-06 Thread Daniel Gustafsson
> On 07 Apr 2018, at 00:23, Andres Freund wrote: > On 2018-04-06 02:28:17 +0200, Daniel Gustafsson wrote: >> Applying this makes the _cancel test pass, moving the failure instead to the >> following _enable test (which matches what coypu and mylodon are seeing). > > FWIW,

Re: Online enabling of checksums

2018-04-06 Thread Robert Haas
On Fri, Apr 6, 2018 at 6:56 PM, Andres Freund wrote: > no one can entirely quibble with the rationale that this is ok (I'll > post a patch cleaning up the atomics simulation of flags in a bit), but > this is certainly not a correct locking strategy. I think we have enough

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 14:33:48 -0700, Andres Freund wrote: > On 2018-04-06 02:28:17 +0200, Daniel Gustafsson wrote: > > Looking into the isolationtester failure on piculet, which builds using > > --disable-atomics, and locust which doesn’t have atomics, the code for > > pg_atomic_test_set_flag seems a bit

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
Hi, On 2018-04-06 02:28:17 +0200, Daniel Gustafsson wrote: > Applying this makes the _cancel test pass, moving the failure instead to the > following _enable test (which matches what coypu and mylodon are seeing). FWIW, I'm somewhat annoyed that I'm now spending time debugging this to get the

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 02:28:17 +0200, Daniel Gustafsson wrote: > Looking into the isolationtester failure on piculet, which builds using > --disable-atomics, and locust which doesn’t have atomics, the code for > pg_atomic_test_set_flag seems a bit odd. > > TAS() is defined to return zero if successful,

Re: Online enabling of checksums

2018-04-06 Thread Tomas Vondra
On 04/06/2018 08:13 PM, Andres Freund wrote: > On 2018-04-06 19:59:17 +0200, Tomas Vondra wrote: >> On 04/06/2018 07:46 PM, Andres Freund wrote: Sure. But what would that be? I can't think of anything. A process that modifies a buffer (or any other piece of shared state) without

Re: Online enabling of checksums

2018-04-06 Thread Tomas Vondra
On 04/06/2018 08:13 PM, Andres Freund wrote: > On 2018-04-06 19:59:17 +0200, Tomas Vondra wrote: >> On 04/06/2018 07:46 PM, Andres Freund wrote: Sure. But what would that be? I can't think of anything. A process that modifies a buffer (or any other piece of shared state) without holding

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 14:14:40 -0400, Robert Haas wrote: > and granted, too, what is Magnus supposed to do about a couple of > committers expressing doubts about whether something really ought to > be committed? Is that an absolute bar? It wasn't phrased as such, > nor do we really have the authority.

Re: Online enabling of checksums

2018-04-06 Thread Robert Haas
On Thu, Apr 5, 2018 at 5:01 PM, Andres Freund wrote: > I've commented weeks ago about my doubts, and Robert concurred: > http://archives.postgresql.org/message-id/CA%2BTgmoZPRfMqZoK_Fbo_tD9OH9PdPFcPBsi-sdGZ6Jg8OMM2PA%40mail.gmail.com Yes, and I expressed some previous

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 19:59:17 +0200, Tomas Vondra wrote: > On 04/06/2018 07:46 PM, Andres Freund wrote: > >> Sure. But what would that be? I can't think of anything. A process that > >> modifies a buffer (or any other piece of shared state) without holding > >> some sort of lock seems broken by default.

Re: Online enabling of checksums

2018-04-06 Thread Tomas Vondra
On 04/06/2018 07:46 PM, Andres Freund wrote: > On 2018-04-06 19:40:59 +0200, Tomas Vondra wrote: >> In any case, I wouldn't call LockBufHdr/UnlockBufHdr a "side channel" >> interlock. It's a pretty direct and intentional interlock, I think. > > I mean it's a side-channel as far as

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 19:40:59 +0200, Tomas Vondra wrote: > In any case, I wouldn't call LockBufHdr/UnlockBufHdr a "side channel" > interlock. It's a pretty direct and intentional interlock, I think. I mean it's a side-channel as far as DataChecksumsNeedWrite() is concerned. You're banking on all callers

Re: Online enabling of checksums

2018-04-06 Thread Tomas Vondra
On 04/06/2018 07:22 PM, Andres Freund wrote: > Hi, > > On 2018-04-06 14:34:43 +0200, Tomas Vondra wrote: >>> Oh, that's not my intention either -- I just wanted to make sure I >>> was thinking about the same issue you were. > >> I agree we shouldn't rely on chance here - if we might read a stale

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
Hi, On 2018-04-06 14:34:43 +0200, Tomas Vondra wrote: > > Oh, that's not my intention either -- I just wanted to make sure I > > was thinking about the same issue you were. > I agree we shouldn't rely on chance here - if we might read a stale > value, we need to fix that of course. It's

Re: Online enabling of checksums

2018-04-06 Thread Andres Freund
On 2018-04-06 11:25:59 +0200, Magnus Hagander wrote: > Since you know a lot more about that type of interlocks than I do :) We > already wait for all running transactions to finish before we start doing > anything. Obviously transactions != buffer writes (and we have things like > the

Re: Online enabling of checksums

2018-04-06 Thread Tomas Vondra
On 04/06/2018 11:25 AM, Magnus Hagander wrote: > > > On Thu, Apr 5, 2018 at 11:41 PM, Andres Freund > wrote: > > Hi, > > On 2018-04-05 23:32:19 +0200, Magnus Hagander wrote: > > On Thu, Apr 5, 2018 at 11:23 PM, Andres Freund

Re: Online enabling of checksums

2018-04-06 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 11:41 PM, Andres Freund wrote: > Hi, > > On 2018-04-05 23:32:19 +0200, Magnus Hagander wrote: > > On Thu, Apr 5, 2018 at 11:23 PM, Andres Freund > wrote: > > > Is there any sort of locking that guarantees that worker processes see >

Re: Online enabling of checksums

2018-04-05 Thread Daniel Gustafsson
> On 05 Apr 2018, at 23:13, Magnus Hagander wrote: > (And yes, we've noticed it's failing in isolationtester on a number of boxes > -- Daniel is currently investigating) Looking into the isolationtester failure on piculet, which builds using --disable-atomics, and locust

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 11:48 PM, Tom Lane wrote: > Magnus Hagander writes: > > I have now pushed this latest version with some minor text adjustments > and > > a catversion bump. > > crake is not happy --- it's failing cross-version upgrade tests

Re: Online enabling of checksums

2018-04-05 Thread Tom Lane
Magnus Hagander writes: > I have now pushed this latest version with some minor text adjustments and > a catversion bump. crake is not happy --- it's failing cross-version upgrade tests because: Performing Consistency Checks - Checking cluster

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
Hi, On 2018-04-05 23:32:19 +0200, Magnus Hagander wrote: > On Thu, Apr 5, 2018 at 11:23 PM, Andres Freund wrote: > > Is there any sort of locking that guarantees that worker processes see > > an up2date value of > > DataChecksumsNeedWrite()/ControlFile->data_checksum_version?

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 11:23 PM, Andres Freund wrote: > Hi, > > On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: > > I have now pushed this latest version with some minor text adjustments > and > > a catversion bump. > > Is there any sort of locking that guarantees that

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
Hi, On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: > I have now pushed this latest version with some minor text adjustments and > a catversion bump. Is there any sort of locking that guarantees that worker processes see an up2date value of

Re: Online enabling of checksums

2018-04-05 Thread Peter Geoghegan
On Thu, Apr 5, 2018 at 1:51 PM, Joshua D. Drake wrote: > Perhaps I am missing something but there has been a lot of public discussion > on this feature for the last 7 weeks of which you barely participated. I > certainly understand wanting some notice before commit but

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 11:09 PM, Peter Geoghegan wrote: > On Thu, Apr 5, 2018 at 1:27 PM, Andres Freund wrote: > >>At least this patch was posted on the lists before commit, unlike many > >>others from many different people. And AFAIK there has never been such

Re: Online enabling of checksums

2018-04-05 Thread Peter Geoghegan
On Thu, Apr 5, 2018 at 1:27 PM, Andres Freund wrote: >>At least this patch was posted on the lists before commit, unlike many >>others from many different people. And AFAIK there has never been such >>a >>rule. The rules cannot possibly anticipate every situation or subtlety.

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
On 2018-04-05 13:51:41 -0700, Joshua D. Drake wrote: > On 04/05/2018 01:12 PM, Andres Freund wrote: > > I want to be on the record that I think merging a nontrival feature that > > got submitted 2018-02-21, just before the start of the last last CF, is > > an abuse of process, and not cool. We've

Re: Online enabling of checksums

2018-04-05 Thread Joshua D. Drake
On 04/05/2018 01:12 PM, Andres Freund wrote: On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: I have now pushed this latest version with some minor text adjustments and a catversion bump. Thanks for all the reviews! I want to be on the record that I think merging a nontrival feature that

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
On April 5, 2018 1:20:52 PM PDT, Magnus Hagander wrote: >On Thu, Apr 5, 2018 at 10:14 PM, Andres Freund >wrote: > >And even worse, without even announcing an intent to commit and giving >> people a chance to object. >> > >At least this patch was posted

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 10:14 PM, Andres Freund wrote: > On 2018-04-05 13:12:08 -0700, Andres Freund wrote: > > On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: > > > I have now pushed this latest version with some minor text adjustments > and > > > a catversion bump. > >

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
On 2018-04-05 13:12:08 -0700, Andres Freund wrote: > On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: > > I have now pushed this latest version with some minor text adjustments and > > a catversion bump. > > > > Thanks for all the reviews! > > I want to be on the record that I think merging

Re: Online enabling of checksums

2018-04-05 Thread Andres Freund
On 2018-04-05 22:06:36 +0200, Magnus Hagander wrote: > I have now pushed this latest version with some minor text adjustments and > a catversion bump. > > Thanks for all the reviews! I want to be on the record that I think merging a nontrival feature that got submitted 2018-02-21, just before

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 7:30 PM, Magnus Hagander wrote: > On Thu, Apr 5, 2018 at 5:08 PM, Andrey Borodin > wrote: > >> >> >> > 5 апр. 2018 г., в 19:58, Magnus Hagander >> написал(а): >> > >> > >> > >> > On Thu, Apr 5, 2018 at 4:55

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 5:08 PM, Andrey Borodin wrote: > > > > 5 апр. 2018 г., в 19:58, Magnus Hagander > написал(а): > > > > > > > > On Thu, Apr 5, 2018 at 4:55 PM, Andrey Borodin > wrote: > > > > > > > 5 апр. 2018 г., в 14:33,

Re: Online enabling of checksums

2018-04-05 Thread Andrey Borodin
> 5 апр. 2018 г., в 19:58, Magnus Hagander написал(а): > > > > On Thu, Apr 5, 2018 at 4:55 PM, Andrey Borodin wrote: > > > > 5 апр. 2018 г., в 14:33, Tomas Vondra > > написал(а): > > > > This patch version seems

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Thu, Apr 5, 2018 at 4:55 PM, Andrey Borodin wrote: > > > > 5 апр. 2018 г., в 14:33, Tomas Vondra > написал(а): > > > > This patch version seems fine to me. I'm inclined to mark it RFC. > +1 > The patch works fine for me. I've tried

Re: Online enabling of checksums

2018-04-05 Thread Andrey Borodin
> 5 апр. 2018 г., в 14:33, Tomas Vondra > написал(а): > > This patch version seems fine to me. I'm inclined to mark it RFC. +1 The patch works fine for me. I've tried different combinations of backend cancelation and the only suspicious thing I found is that you

Re: Online enabling of checksums

2018-04-05 Thread Tomas Vondra
On 4/5/18 11:07 AM, Magnus Hagander wrote: > > > On Wed, Apr 4, 2018 at 12:11 AM, Tomas Vondra > > wrote: > > ... > > It however still fails to initialize the attempts field after allocating > the db entry in

Re: Online enabling of checksums

2018-04-05 Thread Magnus Hagander
On Wed, Apr 4, 2018 at 12:11 AM, Tomas Vondra wrote: > On 04/03/2018 02:05 PM, Magnus Hagander wrote: > > On Sun, Apr 1, 2018 at 2:04 PM, Magnus Hagander > > wrote: > > > > On Sat, Mar 31, 2018 at 5:38 PM, Tomas

Re: Online enabling of checksums

2018-04-04 Thread Michael Banck
Hi, On Tue, Apr 03, 2018 at 02:05:04PM +0200, Magnus Hagander wrote: > PFA a rebase on top of the just committed verify-checksums patch. For the record, I am on vacation this week and won't be able to do further in-depth review or testing of this patch before the end of the commitfest, sorry.

Re: Online enabling of checksums

2018-04-03 Thread Tomas Vondra
On 04/03/2018 02:05 PM, Magnus Hagander wrote: > On Sun, Apr 1, 2018 at 2:04 PM, Magnus Hagander > wrote: > > On Sat, Mar 31, 2018 at 5:38 PM, Tomas Vondra > > >

Re: Online enabling of checksums

2018-04-03 Thread Magnus Hagander
On Sun, Apr 1, 2018 at 2:04 PM, Magnus Hagander wrote: > On Sat, Mar 31, 2018 at 5:38 PM, Tomas Vondra < > tomas.von...@2ndquadrant.com> wrote: > >> On 03/31/2018 05:05 PM, Magnus Hagander wrote: >> > On Sat, Mar 31, 2018 at 4:21 PM, Tomas Vondra >> >

Re: Online enabling of checksums

2018-04-01 Thread Magnus Hagander
On Sat, Mar 31, 2018 at 5:38 PM, Tomas Vondra wrote: > On 03/31/2018 05:05 PM, Magnus Hagander wrote: > > On Sat, Mar 31, 2018 at 4:21 PM, Tomas Vondra > > > > wrote: > > > > ... > > > > I do

Re: Online enabling of checksums

2018-03-31 Thread Magnus Hagander
On Sat, Mar 31, 2018 at 4:21 PM, Tomas Vondra wrote: > > On 03/31/2018 02:02 PM, Magnus Hagander wrote: > > On Sat, Mar 31, 2018 at 2:08 AM, Tomas Vondra > > > > wrote: > > > > > > But wait -

Re: Online enabling of checksums

2018-03-31 Thread Tomas Vondra
Hi, On 03/31/2018 02:02 PM, Magnus Hagander wrote: > On Sat, Mar 31, 2018 at 2:08 AM, Tomas Vondra > > wrote: > > ... > > (a) Should not be difficult to do, I think. We don't have relation_open > with a missing_ok flag,

Re: Online enabling of checksums

2018-03-31 Thread Magnus Hagander
On Sat, Mar 31, 2018 at 2:08 AM, Tomas Vondra wrote: > Hi, > > I've been looking at the patch a bit more, and I think there are a > couple of fairly serious issues in the error handling. > Thanks! > > Firstly ChecksumHelperLauncherMain spends quite a bit of

Re: Online enabling of checksums

2018-03-30 Thread Tomas Vondra
Hi, I've been looking at the patch a bit more, and I think there are a couple of fairly serious issues in the error handling. Firstly ChecksumHelperLauncherMain spends quite a bit of effort on skipping dropped databases, but ChecksumHelperWorkerMain does not do the same thing with tables. I'm

  1   2   >