Re: ci: CCache churns through available space too quickly

Hi,

On 2026-06-08 13:30:03 +0300, Nazir Bilal Yavuz wrote:
> I think it looks okay, no need to use composite actions for this.

Cool.

> I tested the patch and I confirm that it works as mentioned. Here is my review:
>
> All the points you explained in the commit message are nice improvements!
>
> Typo in commit message:
>
> + In my testing this utilizes the available cache space (10GB for personal
> + accounts) much more effictively than before.
>
> Typo at 'effictively' in the commit message.

Oops.

> 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...

> 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...

> 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
>
> +def main():
> + on_default_branch = os.environ["ON_DEFAULT_BRANCH"] == "true"
> + ccache_dir = os.environ["CCACHE_DIR"]
>
> ccache_dir isn't used.

Ah, yea. It was earlier, but I removed that part (computed the cache size,
when this was a shell script, by using du. But that seemed too awkward in
python, so I removed it).

> + # 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".

> , 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?

Greetings,

Andres Freund