CacheServer option (or whatever to work around server error limit)
Hello everybody, I am still struggling with pacman having the server error limit enabled by default. My attempt to fix this for my use case with http headers [0] was reject. Also someone proposed to add a configuration option to disable server error limit [1]... It was rejected as well. But I guess the best way to solve this is finally implementing an option "CacheServer", as requested a long time ago [2]. Looks like Allan did not fine the "spare 30 minutes" [3] to implement... :) Thus I would like to have a look. Still I would like to have a rough guide where to put what and how to glue things. Anybody wants to share some thoughts what an acceptable solution should look like? [0] https://lists.archlinux.org/pipermail/pacman-dev/2021-May/025159.html [1] https://bugs.archlinux.org/task/71352 [2] https://bugs.archlinux.org/task/23407 [3] https://lists.archlinux.org/pipermail/pacman-dev/2021-June/025184.html -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
On 13/1/22 23:11, Christian Hesse wrote:
Hello everybody,
I am still struggling with pacman having the server error limit enabled by default. My attempt to fix this for my use case with http headers [0] was reject.
Also someone proposed to add a configuration option to disable server error limit [1]... It was rejected as well.
But I guess the best way to solve this is finally implementing an option "CacheServer", as requested a long time ago [2]. Looks like Allan did not fine the "spare 30 minutes" [3] to implement... :)
Thus I would like to have a look. Still I would like to have a rough guide where to put what and how to glue things. Anybody wants to share some thoughts what an acceptable solution should look like?
[0] https://lists.archlinux.org/pipermail/pacman-dev/2021-May/025159.html [1] https://bugs.archlinux.org/task/71352 [2] https://bugs.archlinux.org/task/23407 [3] https://lists.archlinux.org/pipermail/pacman-dev/2021-June/025184.html
I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server. Then in the try next server function, check if we are using the cache server, and silently move onto the normal servers. Allan
Allan McRae
On 13/1/22 23:11, Christian Hesse wrote:
Hello everybody,
I am still struggling with pacman having the server error limit enabled by default. My attempt to fix this for my use case with http headers [0] was reject.
Also someone proposed to add a configuration option to disable server error limit [1]... It was rejected as well.
But I guess the best way to solve this is finally implementing an option "CacheServer", as requested a long time ago [2]. Looks like Allan did not fine the "spare 30 minutes" [3] to implement... :)
Thus I would like to have a look. Still I would like to have a rough guide where to put what and how to glue things. Anybody wants to share some thoughts what an acceptable solution should look like?
[0] https://lists.archlinux.org/pipermail/pacman-dev/2021-May/025159.html [1] https://bugs.archlinux.org/task/71352 [2] https://bugs.archlinux.org/task/23407 [3] https://lists.archlinux.org/pipermail/pacman-dev/2021-June/025184.html
I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server.
Sure, so we want to put cache and regular servers in a single list? Currently this uses alpm_list_t, which is a doubly linked list. The servers are added as 'char *' there. Do we want to add a struct for servers which adds a field for storing its type? typedef struct _alpm_server_t { int cache; char *server; } alpm_server_t;
Then in the try next server function, check if we are using the cache server, and silently move onto the normal servers.
-- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
On 14/1/22 00:12, Christian Hesse wrote:
Allan McRae
on Thu, 2022/01/13 23:52: On 13/1/22 23:11, Christian Hesse wrote:
Hello everybody,
I am still struggling with pacman having the server error limit enabled by default. My attempt to fix this for my use case with http headers [0] was reject.
Also someone proposed to add a configuration option to disable server error limit [1]... It was rejected as well.
But I guess the best way to solve this is finally implementing an option "CacheServer", as requested a long time ago [2]. Looks like Allan did not fine the "spare 30 minutes" [3] to implement... :)
Thus I would like to have a look. Still I would like to have a rough guide where to put what and how to glue things. Anybody wants to share some thoughts what an acceptable solution should look like?
[0] https://lists.archlinux.org/pipermail/pacman-dev/2021-May/025159.html [1] https://bugs.archlinux.org/task/71352 [2] https://bugs.archlinux.org/task/23407 [3] https://lists.archlinux.org/pipermail/pacman-dev/2021-June/025184.html
I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server.
Sure, so we want to put cache and regular servers in a single list?
Currently this uses alpm_list_t, which is a doubly linked list. The servers are added as 'char *' there. Do we want to add a struct for servers which adds a field for storing its type?
typedef struct _alpm_server_t { int cache; char *server; } alpm_server_t;
Having not looked at this in detail... my initial reaction is no. Assuming we allow a single CacheServer, would it be more efficient to add the field into the payload struct?
Then in the try next server function, check if we are using the cache server, and silently move onto the normal servers.
Allan McRae
On 14/1/22 00:12, Christian Hesse wrote:
Allan McRae
on Thu, 2022/01/13 23:52: I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server.
Sure, so we want to put cache and regular servers in a single list?
Currently this uses alpm_list_t, which is a doubly linked list. The servers are added as 'char *' there. Do we want to add a struct for servers which adds a field for storing its type?
typedef struct _alpm_server_t { int cache; char *server; } alpm_server_t;
Having not looked at this in detail... my initial reaction is no. Assuming we allow a single CacheServer, would it be more efficient to add the field into the payload struct?
Why limit to just one cache server? For me (with pacredir) this would be sufficient, but I guess others will complain: Having several servers in a network sharing their cache you could add all of them to the configuration: [core] CacheServer = http://server-a.localdomain/ CacheServer = http://server-b.localdomain/ Server = ... -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
Christian Hesse on Thu, 2022/01/13 15:44:
Allan McRae
on Fri, 2022/01/14 00:36: On 14/1/22 00:12, Christian Hesse wrote:
Allan McRae
on Thu, 2022/01/13 23:52: I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server.
Sure, so we want to put cache and regular servers in a single list?
Currently this uses alpm_list_t, which is a doubly linked list. The servers are added as 'char *' there. Do we want to add a struct for servers which adds a field for storing its type?
typedef struct _alpm_server_t { int cache; char *server; } alpm_server_t;
Having not looked at this in detail... my initial reaction is no. Assuming we allow a single CacheServer, would it be more efficient to add the field into the payload struct?
Why limit to just one cache server? For me (with pacredir) this would be sufficient, but I guess others will complain: Having several servers in a network sharing their cache you could add all of them to the configuration:
[core] CacheServer = http://server-a.localdomain/ CacheServer = http://server-b.localdomain/ Server = ...
Currently we have a 'struct server_error_count': struct server_error_count { char server[HOSTNAME_SIZE]; unsigned int errors; }; How about implementing 'struct cache_servers'? struct server_error_count { char server[HOSTNAME_SIZE]; unsigned int errors; }; Or modify to hold both options? struct server_opts { char server[HOSTNAME_SIZE]; unsigned int cache; unsigned int errors; }; -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
Christian Hesse on Thu, 2022/01/13 16:06:
Christian Hesse
on Thu, 2022/01/13 15:44:
Allan McRae
on Fri, 2022/01/14 00:36: On 14/1/22 00:12, Christian Hesse wrote:
Allan McRae
on Thu, 2022/01/13 23:52: I have had a spare 5 minutes... you need to add a CacheServer global option to pacman.conf and wire that into pacman & libalpm. Then when forming the list of Servers in a download payload for a package, you need to add that server at the top and note that it is a cache server.
Sure, so we want to put cache and regular servers in a single list?
Currently this uses alpm_list_t, which is a doubly linked list. The servers are added as 'char *' there. Do we want to add a struct for servers which adds a field for storing its type?
typedef struct _alpm_server_t { int cache; char *server; } alpm_server_t;
Having not looked at this in detail... my initial reaction is no. Assuming we allow a single CacheServer, would it be more efficient to add the field into the payload struct?
Why limit to just one cache server? For me (with pacredir) this would be sufficient, but I guess others will complain: Having several servers in a network sharing their cache you could add all of them to the configuration:
[core] CacheServer = http://server-a.localdomain/ CacheServer = http://server-b.localdomain/ Server = ...
Currently we have a 'struct server_error_count':
struct server_error_count { char server[HOSTNAME_SIZE]; unsigned int errors; };
How about implementing 'struct cache_servers'?
struct server_error_count { char server[HOSTNAME_SIZE]; unsigned int errors; };
Or modify to hold both options?
struct server_opts { char server[HOSTNAME_SIZE]; unsigned int cache; unsigned int errors; };
A friendly ping on this topic... Any more thoughts? -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
From: Christian Hesse
Christian Hesse on Mon, 2022/01/24 00:58:
This implements a new configuration option 'CacheServer'. Adding a cache server makes it ignore the server error limit.
We have a struct that stores the server errors. Extend (and rename) this struct to store if this is a cache server. The errors are not increased for cache servers, thus they are never ignored.
I think storing the cache server state together with the server errors is the only reasonable solution. Extending 'alpm_list_t *servers' would not help a lot as download functions do not have a reference to its server in that list if I got this right. One thing is still missing, though. I think Allen wanted the cache server not to be used for database files. In lib/libalpm/dload.c the code iterated over payload->servers, but only indication there about a database file is its file name (payload->filepath)? Except for the limitation above (which is not an issue for me :) this seems to works really well. Please test, review and comment... -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
From: Christian Hesse
Christian Hesse on Mon, 2022/01/24 12:48:
This implements a new configuration option 'CacheServer'. Adding a cache server makes it ignore the server error limit.
We have a struct that stores the server errors. Extend (and rename) this struct to store if this is a cache server. The errors are not increased for cache servers, thus they are never ignored.
This now skips cache servers for database file downloads. Is there a better way to detect a database file download? BTW, I commented alpm_db_set_servers(). Is that actually used for anything? -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
From: Christian Hesse
Christian Hesse on Mon, 2022/01/31 12:34:
This implements a new configuration option 'CacheServer'. Adding a cache server makes it ignore the server error limit.
We have a struct that stores the server errors. Extend (and rename) this struct to store if this is a cache server. The errors are not increased for cache servers, thus they are never ignored.
I did some polishing... Also log messages are no longer shown (degraded to debug) for server side errors (http code >= 400) with CacheServer. I think this was the initial intention of FS#23407... Wondering why there's no feedback to this patch... Would be nice to receive an ACK or NACK at least on whether or not this is the way to go. I hope the silence is kind of a passive consent. :) -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
On 31/1/22 21:42, Christian Hesse wrote:
Wondering why there's no feedback to this patch... Would be nice to receive an ACK or NACK at least on whether or not this is the way to go.
When two versions were posted within a day, I figured I'd wait a week for v3 before reviewing... And here we are! It is in the review queue - but so are patches from 8 months ago, so don't expect it reviewed any time soon. Allan
Allan McRae
On 31/1/22 21:42, Christian Hesse wrote:
Wondering why there's no feedback to this patch... Would be nice to receive an ACK or NACK at least on whether or not this is the way to go.
When two versions were posted within a day, I figured I'd wait a week for v3 before reviewing... And here we are!
:) Waiting too long with the review could result in me pushing even more patches. ;->
It is in the review queue - but so are patches from 8 months ago, so don't expect it reviewed any time soon.
Oh dear... I hope I can remember the details once review happens. :) Just to remember for myself: Initially I though about making 'alpm_list_t *servers' point to a 'struct alpm_server_t' containing the server and cache state. This does not work well as we have cases in the code where we handle an url that is no longer associated with a server in the list. I think storing the cache state in modified 'struct server_error_count' is the only way to go as we can always extract the server hostname from url. -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];) putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
participants (2)
-
Allan McRae
-
Christian Hesse