Closed
Bug 598098
Opened 14 years ago
Closed 14 years ago
change fuzzer jobs to only work on builds that have symbols available
Categories
(Release Engineering :: General, defect, P4)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: salbiz)
References
Details
Attachments
(1 file, 2 obsolete files)
2.78 KB,
patch
|
catlee
:
review+
catlee
:
feedback+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
* Would prevent fuzzing jobs from pulling a build that doesn't have symbols uploaded yet. (When this happens, the fuzzer thinks all crashes are "new").
* Would prevent build ID collisions from resulting in frankenbuilds (where the symbols don't correspond to the binary). Instead of file overwrites, you'd get an error from the mv command.
Comment 1•14 years ago
|
||
The idea here is to copy files to a temporary directory on the same filesystem as their final location, and then do an atomic rename of that temp directory to the real directory name.
We currently upload files into /tmp, and then copy them one by one into their final location. /tmp is on a different file system than where the builds finally end up.
OS: Mac OS X → All
Priority: -- → P4
Hardware: x86 → All
Comment 2•14 years ago
|
||
jesse and I just talked about this - seems safer to change fuzzing to only download builds that have symbols present at the time of download.
Assignee: nobody → jruderman
Summary: Use directory-rename synchronization when uploading builds → change fuzzer jobs to only work on builds that have symbols available
Comment 3•14 years ago
|
||
(In reply to comment #2)
> jesse and I just talked about this - seems safer to change fuzzing to only
> download builds that have symbols present at the time of download.
Huh? I think the original idea is still valid. There's no way to tell if the builds/symbols being downloaded are complete as it stands right now.
Reporter | ||
Comment 4•14 years ago
|
||
joduinn doesn't want to do mv-synchronization of the entire directory (when the build, tests, and symbols are all uploaded). But you're saying we need to do it at least at the file level, so I don't download an incomplete file?
fwiw I don't know whether I've been downloading incomplete files. I only find out about errors in fuzz jobs when you tell me about them ;)
Comment 5•14 years ago
|
||
Not sure why not. In any case, we should be doing copies from /tmp to a temp file on the same file system, and then an atomic rename to the final dest.
It's possible you've been getting incomplete files...I don't know how apache handles serving files that are currently being written to.
Reporter | ||
Updated•14 years ago
|
Assignee: jruderman → nobody
Assignee | ||
Comment 7•14 years ago
|
||
Initial patch to copy files into a hidden temporary directory in the destination and do an atomic rename once the file is copied onto the filesystem.
Comment 8•14 years ago
|
||
Comment on attachment 494016 [details] [diff] [review]
make post_upload.py use atomic renames
This looks great for individual files.
Can we do atomic directory copies too? i.e. create tmpdir, copy files into it, rename tmpdir to realdir?
Attachment #494016 -
Flags: feedback?(catlee) → feedback+
Assignee | ||
Comment 9•14 years ago
|
||
This will need to be thoroughly tested to make sure it doesn't break anything, which I have not done yet. It uses a single tmpdir to copy the entire directory of files, allowing for a state of atomic copying where we either have all the files completely copied, or none at all.
Attachment #494016 -
Attachment is obsolete: true
Attachment #494835 -
Flags: feedback?(rail)
Attachment #494835 -
Flags: feedback?(catlee)
Comment 10•14 years ago
|
||
Comment on attachment 494835 [details] [diff] [review]
atomic copy directory
>-def CopyFileToDir(original_file, source_dir, dest_dir, preserve_dirs=False):
>+def make_path(path):
>+ """ Ensure that a directory path exists, create it anew if it does not """
>+ if not os.path.isdir(path):
>+ try:
>+ os.makedirs(path, 0755)
>+ except OSError, e:
>+ if e.errno == EEXIST:
>+ print "%s already exists, continuing anyways" % path
>+ else:
>+ raise
>+
make_path should raise an error if path exists but isn't a directory
>+def CopyFiles(files, dest_dir, func_copy):
>+ """ Atomic copy files from one directory to another using a tmpdir to
>+ store intermediate copy, then do an atomic rename. func_copy defines
>+ rules for filtering and operating on each file """
>+ make_path(os.path.dirname(dest_dir))
>+ tmpdir = tempfile.mkdtemp(prefix=".~", dir=os.path.dirname(dest_dir))
>+ for f in files:
>+ func_copy(f, tmpdir)
>+ atomic_rmtree(dest_dir)
>+ os.rename(tmpdir, dest_dir)
So I'm not sure we want to remove dest_dir here...What if files already exist in the destination directory?
Maybe it only makes sense to do atomic directory copies if the directory doesn't already exist. If it does exist, then atomic file copies may be the best we can do.
Attachment #494835 -
Flags: feedback?(catlee) → feedback-
Assignee | ||
Comment 11•14 years ago
|
||
Done, was hoping we could avoid this, but I guess this is the best way to go. Expanded the tests as well.
Attachment #494835 -
Attachment is obsolete: true
Attachment #495094 -
Flags: feedback?(catlee)
Attachment #494835 -
Flags: feedback?(rail)
Assignee | ||
Comment 12•14 years ago
|
||
Comment on attachment 494016 [details] [diff] [review]
make post_upload.py use atomic renames
Based on IRC discussions, I think the consensus was that atomic directory copies don't get us very much and introduce a whack of additional problems, so falling back to using this patch.
Attachment #494016 -
Attachment is obsolete: false
Attachment #494016 -
Flags: review?(catlee)
Assignee | ||
Updated•14 years ago
|
Attachment #495094 -
Attachment is obsolete: true
Attachment #495094 -
Flags: feedback?(catlee)
Updated•14 years ago
|
Attachment #494016 -
Flags: review?(catlee) → review+
Comment 13•14 years ago
|
||
Comment on attachment 494016 [details] [diff] [review]
make post_upload.py use atomic renames
changeset: 1911:9a926a987cdf
Attachment #494016 -
Flags: checked-in+
Comment 14•14 years ago
|
||
Syed, is there anything else to do in this bug?
Assignee | ||
Comment 15•14 years ago
|
||
Should be all done now since we dropped the idea of atomic directory copies.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•