From 5378c38db0a5dedef34fdf35e357f76c104d717a Mon Sep 17 00:00:00 2001
From: Julian Andres Klode <jak@debian.org>
Date: Tue, 24 Jun 2025 16:52:06 +0200
Subject: Fix stuck acquire queues on partial server errors

When a server responds with two InRelease files, but one
is good, and the other indicates a temporary error, we
queued the other indices from the good repository for
downloading and then queued the retry.

The resulting queue ended up with items having fetchAfter=0
before the item with fetchAfter=<something>, causing the
Run() loop to not detect a stuck queue.

Change the order of the queue such that the highest retry-after
items comes first; this ensures that we always see stuck queues.

This of course changes the behavior of retries in that if one
file fails temporarily we block the entire server. This does
seem more beneficial in the common case - if one file fails,
probably all of them fail, and there's no point bombarding
the server with requests for indices from good repositories
until all have failed.

The actual root cause is more that the remaining items are
Enqueued like this:

1. Enqueue jammy/InRelease (delayed)
2. Enqueue jammy-updates/main all Packages

The resulting queue ended up being:

    jammy-updates/main all Packages
    jammy/InRelease (delayed)

But Enqueue() only calls Cycle() when there are no items in
the queue already - after all, any item that is already running
will call Cycle() eventually. Or so was the case until we added
the retry-after handling.

It's unclear why we don't Cycle() all the time when enqueuing
a new item, given that our pipeline might not be filled yet,
and we could send the request to the server while waiting for
data on a running item.

Trying to always Cycle() however led to regressions that still
need investigating. Given that, this solution certainly is the
more easy to reason about one.

LP: #2003851
---
 apt-pkg/acquire.cc                                 |  6 ++---
 .../test-ubuntu-bug-2003851-retry-after            | 29 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)
 create mode 100755 test/integration/test-ubuntu-bug-2003851-retry-after

diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index 41c3d9904..d2d32e52e 100644
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -1031,7 +1031,7 @@ bool pkgAcquire::Queue::Enqueue(ItemDesc &Item)
    };
    QItem **OptimalI = &Items;
    QItem **I = &Items;
-   auto insertLocation = std::make_tuple(Item.Owner->FetchAfter(), -Item.Owner->Priority());
+   auto insertLocation = std::make_tuple(Item.Owner->FetchAfter(), Item.Owner->Priority());
    // move to the end of the queue and check for duplicates here
    for (; *I != 0; ) {
       if (Item.URI == (*I)->URI && MetaKeysMatch(Item, *I))
@@ -1045,10 +1045,10 @@ bool pkgAcquire::Queue::Enqueue(ItemDesc &Item)
       // Determine the optimal position to insert: before anything with a
       // higher priority.
       auto queueLocation = std::make_tuple((*I)->GetFetchAfter(),
-					   -(*I)->GetPriority());
+					   (*I)->GetPriority());
 
       I = &(*I)->Next;
-      if (queueLocation <= insertLocation)
+      if (queueLocation >= insertLocation)
       {
 	 OptimalI = I;
       }
diff --git a/test/integration/test-ubuntu-bug-2003851-retry-after b/test/integration/test-ubuntu-bug-2003851-retry-after
new file mode 100755
index 000000000..492dc2781
--- /dev/null
+++ b/test/integration/test-ubuntu-bug-2003851-retry-after
@@ -0,0 +1,29 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture 'amd64'
+
+insertpackage 'jammy,jammy-updates' 'testpkg' 'all' '1'
+
+setupaptarchive --no-update
+changetowebserver
+
+webserverconfig 'aptwebserver::failrequest' '503'
+webserverconfig 'aptwebserver::failrequest::dists/jammy/InRelease' '4'
+testwarningequal "Delaying http://localhost:${APTHTTPPORT} jammy InRelease by 1 seconds
+Ign:1 http://localhost:${APTHTTPPORT} jammy InRelease
+  503  Service Unavailable
+Get:2 http://localhost:${APTHTTPPORT} jammy-updates InRelease [1398 B]
+Err:1 http://localhost:${APTHTTPPORT} jammy InRelease
+  503  Service Unavailable
+Get:3 http://localhost:${APTHTTPPORT} jammy-updates/main all Packages [257 B]
+Get:4 http://localhost:${APTHTTPPORT} jammy-updates/main Translation-en [235 B]
+Reading package lists...
+Building dependency tree...
+All packages are up to date.
+W: Failed to fetch http://localhost:${APTHTTPPORT}/dists/jammy/InRelease  503  Service Unavailable
+W: Some index files failed to download. They have been ignored, or old ones used instead." apt update -o acquire::retries=1 -o debug::acquire::retries=1
-- 
cgit v1.2.3-70-g09d2