Hi,
On Mon, 8 Jun 2026 at 17:59, Andres Freund <[email protected]> wrote:
>
> > diff --git a/.github/workflows/pg-ci.yml b/.github/workflows/pg-ci.yml
> > index 8560e9389f6..86dc47de8db 100644
> > --- a/.github/workflows/pg-ci.yml
> > +++ b/.github/workflows/pg-ci.yml
> >
> > + - &ccache_decide_save_step
> > + name: "ccache: Decide if cache should be uploaded"
> > + id: ccache-pre-save
> > + # [Decide to] store the cache whenever the cache was set up, so
> > that
> > + # incrementally addressing compiler errors/warnings doesn't have to
> > + # start from scratch.
> > + if: |
> > + always() &&
> > + steps.ccache-restore-branch.conclusion == 'success'
> > + run: python3 src/tools/ci/gha_ccache_decide.py
> >
> > Isn't the conclusion always true unless GitHub has some self errors?
>
> I mean, the cache restoration *could* fail? Or another earlier step could We
> don't want to upload a new cache entry if we never got to building...
I see, yes these points make sense.
> > Also, we are directly running this script with the 'python3' command
> > but it might not be available on the PATH. I had some problems with
> > this on BSD images when we were using Cirrus. I am not sure we would
> > have such problems with GitHub Actions but I just wanted to mention
> > it.
>
> I think we'll just have to address it if/when it becomes a problem. I don't
> really see the alternative...
Sounds good.
> > diff --git a/src/tools/ci/gha_ccache_decide.py
> > b/src/tools/ci/gha_ccache_decide.py
> > new file mode 100644
> > index 00000000000..920f7bf9685
> > --- /dev/null
> > +++ b/src/tools/ci/gha_ccache_decide.py
>
> > + # If there were either barely any misses, or the cache hit ratio was
> > high,
> > + # there no point in generating a new cache entry. We have limited cache
> > + # space.
> > + should_save = misses > 10 and hit_pct < target_rate
> >
> > We consider misses here but we don't mention it
>
> I was trying to mention it, via "If there were either barely any misses".
Sorry, what I meant was we don't mention in the logs, which is:
+ if not should_save:
+ print(f"hit rate {hit_pct} is above target of {target_rate},
skip creating new cache entry")
+ return 0
> > , we only mention hit rate and target rate. I think this is not very
> > important since we can't possibly have a case that misses < 10 and hit_pct <
> > target_rate.
>
> Why could we not have such a case? If we start building with some changes
> that trigger cache misses, but there's a compiler error a few seconds in, that
> seems like it'd precisely hit that case?
Yes, you are right. I hadn't thought of the failure case. Then, it
would be good to mention that case in the log I mentioned above.
Otherwise, we will be printing the incorrect reason.
--
Regards,
Nazir Bilal Yavuz
Microsoft
