Re: fix for partial reads from cache
Raven Kopelman <[email protected]> Tue, 27 Oct 2020 13:02:17 -0700
| Newsgroups | gmane.comp.programming.tools.scons.devel |
|---|---|
| Message-ID | <CAHoKKaoFxftBr+tjQnAffb9UG0S-JxPg1Wf-_UFCpH9_8y9NBQ@mail.gmail.com> |
--===============8830542655102533354==
Content-Type: multipart/alternative; boundary="000000000000c7cc6605b2ac867a"
--000000000000c7cc6605b2ac867a
Content-Type: text/plain; charset="UTF-8"
Looks like attachment is not easy to access; here's plaintext patch
diff -ru -x '*.pyc' SCons.orig/CacheDir.py
/usr/local/lib/python2.7/dist-packages/scons/SCons/CacheDir.py
--- SCons.orig/CacheDir.py 2020-10-23 16:12:44.773349285 -0700
+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/CacheDir.py
2020-10-23 16:19:48.676570382 -0700
@@ -56,7 +56,20 @@
if fs.islink(cachefile):
fs.symlink(fs.readlink(cachefile), t.get_internal_path())
else:
- env.copy_from_cache(cachefile, t.get_internal_path())
+ try:
+ env.copy_from_cache(cachefile, t.get_internal_path())
+ except:
+ try:
+ # In case file was partially retrieved (and now
corrupt)
+ # delete it to avoid poisoning commands like 'ar' that
+ # read from the initial state of the file they are
writing
+ # to.
+ t.fs.unlink(t.get_internal_path())
+ except:
+ pass
+
+ raise
+
try:
os.utime(cachefile, None)
except OSError:
@@ -71,7 +84,7 @@
cd = env.get_CacheDir()
cachedir, cachefile = cd.cachepath(t)
if t.fs.exists(cachefile):
- return "Retrieved `%s' from cache" % t.get_internal_path()
+ return "Retrieving `%s' from cache" % t.get_internal_path()
return None
CacheRetrieve = SCons.Action.Action(CacheRetrieveFunc, CacheRetrieveString)
diff -ru -x '*.pyc' SCons.orig/Taskmaster.py
/usr/local/lib/python2.7/dist-packages/scons/SCons/Taskmaster.py
--- SCons.orig/Taskmaster.py 2020-10-23 16:12:44.785349378 -0700
+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/Taskmaster.py
2020-10-23 15:55:41.521758762 -0700
@@ -243,10 +243,13 @@
cached_targets.append(t)
if len(cached_targets) < len(self.targets):
# Remove targets before building. It's possible that we
- # partially retrieved targets from the cache, leaving
- # them in read-only mode. That might cause the command
+ # retrieved a subset of targets from the cache, leaving
+ # them in an inconsistent state. That might cause the
command
# to fail.
#
+ # Note that retrieve_from_cache() ensures no single target
can
+ # be partially retrieved (file left in corrupt state).
+ #
for t in cached_targets:
try:
t.fs.unlink(t.get_internal_path())
--
*Raven Kopelman* | Team Lead, Senior Developer
Safe Software Inc.
*T* 604.501.9985 x 331 | *F* 604.501.9965
[email protected] | www.safe.com
<http://www.safe.com/emailsignature>
On Tue, Oct 27, 2020 at 1:00 PM Raven Kopelman <[email protected]>
wrote:
> Hi scons devs,
>
> Here's a fix to clean up reads from the cache that fail part way through.
> Absolutely experienced by us in production (non-local shared cache). This
> may be what my last UUID temp name fix was really trying to address, but I
> can imagine edge cases that the UUID fix could still improve.
>
> This could also have been fixed in Taskmaster.py near my comment change,
> but the reasons for cleanup seemed different enough to keep them distinct.
>
> Also cleaned up the logged message tense, as message is logged before the
> cache retrieval begins.
>
> Thanks!
> --
> *Raven Kopelman* | Team Lead, Senior Developer
>
> Safe Software Inc.
> *T* 604.501.9985 x 331 | *F* 604.501.9965
> [email protected] | www.safe.com
>
> <http://www.safe.com/emailsignature>
>
--000000000000c7cc6605b2ac867a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div>Looks like attachment is not easy to access; here'=
;s plaintext patch</div><div><br></div><div>diff -ru -x '*.pyc' SCo=
ns.orig/CacheDir.py /usr/local/lib/python2.7/dist-packages/scons/SCons/Cach=
eDir.py<br>--- SCons.orig/CacheDir.py 2020-10-23 16:12:44.773349285 -0700<b=
r>+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/CacheDir.py 2020-1=
0-23 16:19:48.676570382 -0700<br>@@ -56,7 +56,20 @@<br>=C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0if fs.islink(cachefile):<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0fs.symlink(fs.readlink(cachefile), t.get_internal_path())<=
br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else:<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0env.copy_from_cache(cachefile, t.get_internal_path())<br>+ =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0try:<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env.copy_from_cache(cachefile, t.get_interna=
l_path())<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0except:<br>+ =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0try:<br>+ =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# In case file was p=
artially retrieved (and now corrupt)<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# delete it to avoid poisoning comman=
ds like 'ar' that<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0# read from the initial state of the file they a=
re writing<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0# to.<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0t.fs.unlink(t.get_internal_path())<br>+ =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0except:<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pass<br>+<br>+ =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0raise<br>+<br>=C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0 =C2=A0try:<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0os.utime(cachefile, None)<br>=C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0except OSError:<br>@@ -71,7 +84,7 @@<br>=C2=A0 =
=C2=A0 =C2=A0cd =3D env.get_CacheDir()<br>=C2=A0 =C2=A0 =C2=A0cachedir, cac=
hefile =3D cd.cachepath(t)<br>=C2=A0 =C2=A0 =C2=A0if t.fs.exists(cachefile)=
:<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0return "Retrieved `%s' from cach=
e" % t.get_internal_path()<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0return &quo=
t;Retrieving `%s' from cache" % t.get_internal_path()<br>=C2=A0 =
=C2=A0 =C2=A0return None<br>=C2=A0<br>=C2=A0CacheRetrieve =3D SCons.Action.=
Action(CacheRetrieveFunc, CacheRetrieveString)<br>diff -ru -x '*.pyc=
9; SCons.orig/Taskmaster.py /usr/local/lib/python2.7/dist-packages/scons/SC=
ons/Taskmaster.py<br>--- SCons.orig/Taskmaster.py 2020-10-23 16:12:44.78534=
9378 -0700<br>+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/Taskma=
ster.py 2020-10-23 15:55:41.521758762 -0700<br>@@ -243,10 +243,13 @@<br>=C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cached_targets.a=
ppend(t)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if len(cached_t=
argets) < len(self.targets):<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0# Remove targets before building. It's possible=
that we<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# part=
ially retrieved targets from the cache, leaving<br>- =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# them in read-only mode. That might caus=
e the command<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0#=
retrieved a subset of targets from the cache, leaving<br>+ =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# them in an inconsistent state. T=
hat might cause the command<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0# to fail.<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
=C2=A0 =C2=A0 =C2=A0#<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0# Note that retrieve_from_cache() ensures no single target can<br=
>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# be partially re=
trieved (file left in corrupt state).<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0#<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0for t in cached_targets:<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0try:<br>=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0t.=
fs.unlink(t.get_internal_path())</div><div><br></div><div><div><div dir=3D"=
ltr" class=3D"gmail_signature" data-smartmail=3D"gmail_signature"><div dir=
=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr">=
<div><div dir=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr"><div><div=
dir=3D"ltr"><div><div dir=3D"ltr">--<br><b>Raven Kopelman</b> | Team Lead,=
Senior Developer<br><br><div dir=3D"ltr">Safe Software Inc.</div><b>T</b> =
604.501.9985 x 331 | <b>F</b> 604.501.9965</div><div dir=3D"ltr"><a href=3D=
"mailto:[email protected]" target=3D"_blank">[email protected]<=
/a> | <a href=3D"http://www.safe.com" target=3D"_blank">www.safe.com</a></d=
iv><div dir=3D"ltr"><div><br></div><div><div><a href=3D"http://www.safe.com=
/emailsignature" target=3D"_blank"><img src=3D"http://cdn.safe.com/emailsig=
nature.jpg"></a><br></div></div></div></div></div></div></div></div></div><=
/div></div></div></div></div></div></div></div></div></div></div></div><br>=
</div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_=
attr">On Tue, Oct 27, 2020 at 1:00 PM Raven Kopelman <<a href=3D"mailto:=
[email protected]">[email protected]</a>> wrote:<br></div><b=
lockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-le=
ft:1px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div>Hi sc=
ons devs,</div><div><br></div><div>Here's a fix to clean up reads from =
the cache that fail part way through.=C2=A0 Absolutely experienced by us in=
production (non-local shared cache).=C2=A0 This may be what my last UUID t=
emp name fix was really trying to address, but I can imagine edge cases tha=
t the UUID fix could still improve.</div><div><br></div><div>This could als=
o have been fixed in Taskmaster.py near my comment change, but the reasons =
for cleanup seemed different enough to keep them distinct.<br></div><div><b=
r></div><div>Also cleaned up the logged message tense, as message is logged=
before the cache retrieval begins.</div><div><br></div><div>Thanks!<br></d=
iv><div><div><div dir=3D"ltr"><div dir=3D"ltr"><div><div dir=3D"ltr"><div><=
div dir=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr"><div><div dir=
=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr"><div><div dir=3D"ltr">=
--<br><b>Raven Kopelman</b> | Team Lead, Senior Developer<br><br><div dir=
=3D"ltr">Safe Software Inc.</div><b>T</b> 604.501.9985 x 331 | <b>F</b> 604=
.501.9965</div><div dir=3D"ltr"><a href=3D"mailto:[email protected]" =
target=3D"_blank">[email protected]</a> | <a href=3D"http://www.safe.=
com" target=3D"_blank">www.safe.com</a></div><div dir=3D"ltr"><div><br></di=
v><div><div><a href=3D"http://www.safe.com/emailsignature" target=3D"_blank=
"><img src=3D"http://cdn.safe.com/emailsignature.jpg"></a><br></div></div><=
/div></div></div></div></div></div></div></div></div></div></div></div></di=
v></div></div></div></div></div></div></div></div>
</blockquote></div>
--000000000000c7cc6605b2ac867a--
--===============8830542655102533354==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Scons-dev mailing list
[email protected]
https://pairlist2.pair.net/mailman/listinfo/scons-dev
--===============8830542655102533354==--