PR#2082: delete oldest failed buildroot, when there is no space - koji

koji

#2082 delete oldest failed buildroot, when there is no space

Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.

tkopecek/koji issue1913 into master

remove unneeded condition

Tomas Kopecek • 4 years ago

7457e31

delete main part of data

Tomas Kopecek • 4 years ago

f041fd3

add real delete

Tomas Kopecek • 4 years ago

f95a3a3

delete oldest failed buildroot, when there is no space

Tomas Kopecek • 4 years ago

8023fde

koji/daemon.py

file modified

+6 -3

		`@@ -683,7 +683,8 @@`
		`# index`
		`missed_br = dict([(row['id'], row) for row in missed_br])`
		`tasks = dict([(row['id'], row) for row in self.session.getTaskInfo(tasks)])`
		`- for id in local_only:`
		`+ # go from +- oldest`
		`+ for id in sorted(local_only):`
		`# Cleaning options`
		`# - wait til later`
		`# - "soft" clean (leaving empty root/ dir)`
		`@@ -708,8 +709,10 @@`
		`age < self.options.failed_buildroot_lifetime:`
		`# XXX - this could be smarter`
		`# keep buildroots for failed tasks around for a little while`
		`- self.logger.debug("Keeping failed buildroot: %s" % desc)`
		`- continue`
		`+ if self.checkSpace():`
		`+ # we can leave it in place, otherwise delete it`
		`+ self.logger.debug("Keeping failed buildroot: %s" % desc)`
		`+ continue`
		`topdir = data['dir']`
		`rootdir = None`
		`if topdir:`

tkopecek commented 4 years ago

Fixes: https://pagure.io/koji/issue/1913

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

4 years ago

1 new commit added

add real delete

4 years ago

1 new commit added

delete main part of data

4 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

4 years ago

mikem commented 4 years ago

elif age > 120 or not self.checkSpace():

It's not clear to me why we need to change this condition. It's at least worth a comment.

I think this 2 minute barrier may be a protection against a race condition. Are we worried that 2 minutes is too long for an old buildroot to persist?

Edited 4 years ago by mikem

tkopecek commented 4 years ago

Ah, it is probably not needed now. I've added that check, while Martin tested it. But it showed as a false alarm. (removing)

1 new commit added