519229 - purge_builds.py shouldn't count the current build directory as "used"

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

I've noticed that even after two builds in a row from the exact same builder happen on the same slave, sometimes purge_builds.py tries to clean up.

Comment 1

[Chris Cooper [:coop] (he/him)]

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 2

[Aki Sasaki (not active)]

I'm really tempted to wontfix this.
Is it breaking anything?

Comment 3

[[e-mail] (:bhearsum)]

It makes us clobber more build directories than necessary, which probably hurts our build times overall.

Comment 4

[Armen [:armenzg]]

bhearsum is this still wanted?

Comment 5

[[e-mail] (:bhearsum)]

Yes, this still causes unnecessary clobbers.

Comment 6

[Chris Cooper [:coop] (he/him)]

(In reply to comment #3)
> It makes us clobber more build directories than necessary, which probably hurts
> our build times overall.

Trying to make sure I understand...the change you're proposing is to consider free space as reported free space + current build dir used space? 

Is calculating the used space of the current build dir for every build faster or slower than just clobbering?

Comment 7

[[e-mail] (:bhearsum)]

(In reply to comment #6)
> (In reply to comment #3)
> > It makes us clobber more build directories than necessary, which probably hurts
> > our build times overall.
> 
> Trying to make sure I understand...the change you're proposing is to consider
> free space as reported free space + current build dir used space? 

For the purposes of purge_builds.py, yeah.

> Is calculating the used space of the current build dir for every build faster
> or slower than just clobbering?

Not sure, but full clobbers on 64-bit Mac are significantly longer than a dep build.

We could skip calculating the disk space of the current directory in any case where we already have enough free space without taking it into account.

If you want to argue that it's not worth the effort I'm not going to push back.

Comment 8

[Chris Cooper [:coop] (he/him)]

(In reply to comment #7)
> We could skip calculating the disk space of the current directory in any case
> where we already have enough free space without taking it into account.

Bingo! I think that makes this bug much easier to implement and worth pursuing.

We'd just need to add a new flag to purge_builds.py to take the current dir as a special arg (which we can then check if necessary) rather than just adding it as another protected dir at the end of the list.

Comment 9

[Chris Cooper [:coop] (he/him)]

Attachment: Purge cwd after everything else, but only if we still need to

I'd tried this a few ways with actually calculating the space we'd gain for purging cwd, but it was taking far too long to calculate the space used vs. just removing the dir. I figure if you don't have enough free space before the final dir purge (cwd) and you *still* don't after, you've got bigger problems anyway, so it's easier to just try that final delete and let the step fail if it's going to.

I do skip over deleting the tools/ dir under the current dir. Because of our current order of operations, the tools/ dir gets cloned before the clobber happens. Maybe we want to fix up that logic too, but this patch avoids deleting the new clone.

I'll need to make a follow-up patch to buildbotcustom to stop adding cwd to the skip list because this will make it redundant.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 502898 [details] [diff] [review]
Purge cwd after everything else, but only if we still need to

Sounds great to me!

Comment 11

[Chris Cooper [:coop] (he/him)]

Attachment: Stop passing cwd as a dir to be ignored

(In reply to comment #9)
> I'll need to make a follow-up patch to buildbotcustom to stop adding cwd to the
> skip list because this will make it redundant.

...and here it is.

Comment 12

[Chris Cooper [:coop] (he/him)]

Comment on attachment 502898 [details] [diff] [review]
Purge cwd after everything else, but only if we still need to

https://hg.mozilla.org/build/tools/rev/df1d119be141

Comment 13

[Chris Cooper [:coop] (he/him)]

Comment on attachment 502904 [details] [diff] [review]
Stop passing cwd as a dir to be ignored

https://hg.mozilla.org/build/buildbotcustom/rev/8344247b0811

Comment 14

[Chris Cooper [:coop] (he/him)]

At some point we'll need a reconfig to pick up the buildbotcustom change (landed), but it's very low priority and can easily be batched with other landings the next time someone needs to merge to production-0.8.