After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 707157 - traceback from partial fetch code
traceback from partial fetch code
Status: RESOLVED FIXED
Product: ostree
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: OSTree maintainer(s)
OSTree maintainer(s)
: 707300 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-08-30 23:02 UTC by Colin Walters
Modified: 2013-09-02 18:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fetcher: Clean up code to avoid intermediate files (15.75 KB, patch)
2013-08-30 23:37 UTC, Colin Walters
committed Details | Review
fetcher: Only open files when we are ready to write to them (7.45 KB, patch)
2013-08-31 14:51 UTC, Colin Walters
committed Details | Review
libostree: Change synchronous fetching API to return a stream (13.82 KB, patch)
2013-08-31 15:25 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2013-08-30 23:02:22 UTC
Receiving objects: 6% (64/989) 294.6 kB[Thread 0x7ffff4c7a700 (LWP 4729) exited]

(ostree pull:4725): GLib-GIO-CRITICAL **: g_simple_async_result_take_error: assertion `G_IS_SIMPLE_ASYNC_RESULT (simple)' failed

Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=0x7ffff7683872 "GLib-GIO", log_level=G_LOG_LEVEL_CRITICAL, format=<value optimized out>, args=<value optimized out>) at gmessages.c:981
981		  g_private_set (&g_log_depth, GUINT_TO_POINTER (depth));
Missing separate debuginfos, use: debuginfo-install bzip2-libs-1.0.5-7.el6_0.x86_64 libacl-2.2.49-6.el6.x86_64 libarchive-2.8.3-4.el6_2.x86_64 libattr-2.4.44-7.el6.x86_64 libffi-3.0.5-3.2.el6.x86_64 libselinux-2.0.94-5.3.el6_4.1.x86_64 openssl-1.0.1e-8.el6.x86_64 sqlite-3.6.20-1.el6.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-29.el6.x86_64
(gdb)  bt
  • #0 g_logv
    at gmessages.c line 981
  • #1 g_log
    at gmessages.c line 1010
  • #2 ostree_fetcher_request_uri_with_partial_async
    at /src/ostree/src/libostree/ostree-fetcher.c line 419
  • #3 on_metadata_objects_to_fetch_ready
    at /src/ostree/src/libostree/ostree-repo-pull.c line 967
  • #4 g_main_dispatch
    at gmain.c line 3054
  • #5 g_main_context_dispatch
    at gmain.c line 3630
  • #6 g_main_context_iterate
    at gmain.c line 3701
  • #7 g_main_loop_run
    at gmain.c line 3895
  • #8 run_mainloop_monitor_fetcher
    at /src/ostree/src/libostree/ostree-repo-pull.c line 318
  • #9 ostree_repo_pull
    at /src/ostree/src/libostree/ostree-repo-pull.c line 1321
  • #10 ostree_builtin_pull
    at /src/ostree/src/ostree/ot-builtin-pull.c line 64
  • #11 ostree_run
    at /src/ostree/src/ostree/ot-main.c line 254
  • #12 main
    at /src/ostree/src/ostree/main.c line 69
  • #1 g_log
    at gmessages.c line 1010
  • #2 ostree_fetcher_request_uri_with_partial_async
    at /src/ostree/src/libostree/ostree-fetcher.c line 419
$1 = (OstreeFetcherPendingURI *) 0xb17a30
(gdb) p *$
$2 = {refcount = 2, self = 0x63dc50, uri = 0xb16e00, state = OSTREE_FETCHER_STATE_PENDING, request = 0xb03b20, tmpfile = 0x7ffff00b7f60, 
  filename = 0xb19a80 "295406e24543834532859ebb2014666c3cc49dfd52d6be66ccf4b4d205306b14", request_body = 0x0, out_stream = 0x0, content_length = 0, cancellable = 0x0, result = 0x0}
(gdb) p pending->result
$3 = (GSimpleAsyncResult *) 0x0
Comment 1 Colin Walters 2013-08-30 23:37:09 UTC
Created attachment 253665 [details] [review]
fetcher: Clean up code to avoid intermediate files

I think originally we had the .part/.done separation because we were
trying to support partial downloads of files like repo/config and
repo/refs.

But now that the http server configuration won't give us partial
results, we don't need to support caching those files between runs.

And thus, there's no reason to have the .part/.done and do the dance
with renaming them.

When fetching objects/ and other things that use _with_async, we
continue to use _append_to(), and if the returned range tells us we
have all the bytes, then we hand the full file over to the caller.

Don't attempt to shortcut in the case where the last run told us we
already have the object; the object fetcher code will not make a
request.

While we're here, also clean up use of GError and consistently use the
cancellable from the pending.
Comment 2 Colin Walters 2013-08-30 23:38:11 UTC
But this is still buggy - we have a file descriptor open per HTTP request, which for large pulls quickly causes us to abort.

Need to only open the tmpfile once we actually start getting results back from the server.
Comment 3 Colin Walters 2013-08-31 14:51:57 UTC
Created attachment 253688 [details] [review]
fetcher: Only open files when we are ready to write to them

Otherwise we quickly run out of file descriptors when doing large
requests.
Comment 4 Colin Walters 2013-08-31 15:25:18 UTC
Created attachment 253691 [details] [review]
libostree: Change synchronous fetching API to return a stream

There's not a good reason to write small things such as repo/config to
the filesystem, only to read them back in again.  Change the
non-partial API to just return a stream, then read it into a memory
buffer.
Comment 5 Jeremy Whiting 2013-09-02 18:40:52 UTC
Looks good to me. go for it.
Comment 6 Colin Walters 2013-09-02 18:47:48 UTC
*** Bug 707300 has been marked as a duplicate of this bug. ***
Comment 7 Colin Walters 2013-09-02 18:53:16 UTC
Attachment 253665 [details] pushed as 8b5f684 - fetcher: Clean up code to avoid intermediate files
Attachment 253688 [details] pushed as 95a7854 - fetcher: Only open files when we are ready to write to them
Attachment 253691 [details] pushed as 597da6c - libostree: Change synchronous fetching API to return a stream