Skip to content

Commit

Permalink
fetcher: Drop max queue size assertion in libsoup/libcurl backends
Browse files Browse the repository at this point in the history
Since f4d1334 the primary pull code maintains a
maximum queue. In that commit message I said `Note that I kept an assertion.`.
But I think this is wrong since while it covers a lot of the normal cases, if
one is e.g. trying to fetch a ton of refs, the primary pull code doesn't yet
queue those.  While it'd be nice to queue those, it isn't worth carrying
extra assertions in the backends that can still trigger.

Closes: #1451

Closes: #1453
Approved by: dbnicholson
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Feb 14, 2018
1 parent 8dd68fb commit 96eec98
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
6 changes: 0 additions & 6 deletions src/libostree/ostree-fetcher-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -866,12 +866,6 @@ _ostree_fetcher_request_async (OstreeFetcher *self,
initiate_next_curl_request (req, task);

g_hash_table_add (self->outstanding_requests, g_steal_pointer (&task));

/* Sanity check, I added * 2 just so we don't abort if something odd happens,
* but we do want to abort if we're asked to do obviously too many requests.
*/
g_assert_cmpint (g_hash_table_size (self->outstanding_requests), <,
_OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS * 2);
}

void
Expand Down
14 changes: 8 additions & 6 deletions src/libostree/ostree-fetcher-soup.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ typedef struct {
int base_tmpdir_dfd;

GVariant *extra_headers;
int max_outstanding;
gboolean transfer_gzip;

/* Our active HTTP requests */
Expand Down Expand Up @@ -386,8 +385,6 @@ start_pending_request (ThreadClosure *thread_closure,
OstreeFetcherPendingURI *pending;
GCancellable *cancellable;

g_assert_cmpint (g_hash_table_size (thread_closure->outstanding), <, thread_closure->max_outstanding);

pending = g_task_get_task_data (task);
cancellable = g_task_get_cancellable (task);

Expand Down Expand Up @@ -473,7 +470,6 @@ ostree_fetcher_session_thread (gpointer data)
{
ThreadClosure *closure = data;
g_autoptr(GMainContext) mainctx = g_main_context_ref (closure->main_context);
gint max_conns;

/* This becomes the GMainContext that SoupSession schedules async
* callbacks and emits signals from. Make it the thread-default
Expand All @@ -494,17 +490,23 @@ ostree_fetcher_session_thread (gpointer data)

/* XXX: Now that we have mirrorlist support, we could make this even smarter
* by spreading requests across mirrors. */
gint max_conns;
g_object_get (closure->session, "max-conns-per-host", &max_conns, NULL);
if (max_conns < _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS)
{
/* We download a lot of small objects in ostree, so this
* helps a lot. Also matches what most modern browsers do. */
* helps a lot. Also matches what most modern browsers do.
*
* Note since https://github.com/ostreedev/ostree/commit/f4d1334e19ce3ab2f8872b1e28da52044f559401
* we don't do queuing in this libsoup backend, but we still
* want to override libsoup's currently conservative
* #define SOUP_SESSION_MAX_CONNS_PER_HOST_DEFAULT 2 (as of 2018-02-14).
*/
max_conns = _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS;
g_object_set (closure->session,
"max-conns-per-host",
max_conns, NULL);
}
closure->max_outstanding = 3 * max_conns;

/* This model ensures we don't hit a race using g_main_loop_quit();
* see also what pull_termination_condition() in ostree-repo-pull.c
Expand Down

0 comments on commit 96eec98

Please sign in to comment.