Re: Adding CI to our tree - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Adding CI to our tree
Date
Msg-id 20220213012008.4wpqzyszxu3z22d2@alap3.anarazel.de
Whole thread Raw
In response to Re: Adding CI to our tree  (Justin Pryzby <pryzby@telsasoft.com>)
Responses Re: Adding CI to our tree  (Justin Pryzby <pryzby@telsasoft.com>)
List pgsql-hackers
Hi,

Alvaro adding you because the "branch" discussion in the MERGE thread.


On 2022-02-03 23:04:04 -0600, Justin Pryzby wrote:
> > I'm a bit worried about the increased storage and runtime overhead due to the
> > docs changes. We probably can make it a good bit cheaper though.
>
> If you mean overhead of additional disk operations, it shouldn't be an issue,
> since the git clone uses shared references (not even hardlinks).

I was concerned about those and just the increased runtime of the script. Our
sources are 130MB, leaving the shared .git aside. But maybe it's just fine.

We probably can just get rid of the separate clone and configure run though?
Build the docs, copy the output, do a git co parent docs/, build again?


What was the reason behind moving the docs stuff from the compiler warnings
task to linux? Not that either fits very well... I think it might be worth
moving the docs stuff into its own task, using a 1 CPU container (docs build
isn't parallel anyway). Think that'll be easier to see in the cfbot page. I
think it's also good to run it independent of the linux task succeeding - a
docs failure seems like a separate enough issue.


> > > 9d0f03d3450 wip: cirrus: code coverage
> >
> > I don't think it's good to just unconditionally reference the master branch
> > here. It'll do bogus stuff once 15 is branched off. It works for cfbot, but
> > not other uses.
>
> That's only used for filtering changed files.  It uses git diff --cherry-pick
> postgres/master..., which would *try* to avoid showing anything which is also
> in master.

You commented in another email on this:

On 2022-02-11 12:51:50 -0600, Justin Pryzby wrote:
> Because I put your patch on top of some other branch with the CI coverage (and
> other stuff).
>
> It tries to only show files changed by the branch being checked:
> https://github.com/justinpryzby/postgres/commit/d668142040031915
>
> But it has to figure out where the branch "starts".  Which I did by looking at
> "git diff --cherry-pick origin..."
>
> Andres thinks that does the wrong thing if CI is run manually (not by CFBOT)
> for patches against backbranches.  I'm not sure git diff --cherry-pick is
> widely known/used, but I think using that relative to master may be good
> enough.

Note that I'm not concerned about "manually" running CI against other branches
- I'm concerned about the point where where 15 branches off and CI will
automatically also run against 15. E.g. in the postgres repo
https://cirrus-ci.com/github/postgres/postgres/

I can see a few ways to deal with this:
1) iterate over release branches and see which has the smallest diff
2) parse the branch name, if it's a cfbot run, we know it's master, otherwise skip
3) change cfbot so that it maintains things as pull requests, which have a
   base branch


> > Is looking at the .c files in the change really a reliable predictor of where
> > code coverage changes? I'm doubtful. Consider stuff like removing the last
> > user of some infrastructure or such. Or adding the first.
>
> You're right that it isn't particularly accurate, but it's a step forward if
> lots of patches use it to check/improve coverge of new code.

Maybe it's good enough... The overhead in test runtime is noticeable (~5.30m
-> ~7.15m), but probably acceptable. Although I also would like to enable
-fsanitize=alignment and -fsanitize=alignment, which add about 2 minutes as
well (-fsanitize=address is a lot more expensive), they both work best on
linux.


> In addition to the HTML generated for changed .c files, it's possible to create
> a coverage.gcov output for everything, which could be retrieved to generate
> full HTML locally.  It's ~8MB (or 2MB after gzip).

Note sure that doing doing it locally adds much over just running tests
locally.

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: FailedAssertion("prev_first_lsn < cur_txn->first_lsn", File: "reorderbuffer.c", Line: 1173, ...
Next
From: Andres Freund
Date:
Subject: Re: Why is src/test/modules/committs/t/002_standby.pl flaky?