Add `load_balance_hosts` parameter to db config by dpirotte · Pull Request #736 · pgbouncer/pgbouncer

dpirotte · 2022-07-05T15:14:30Z

PGBouncer's default behavior is to round-robin between comma-separated
hosts on each new connection attempt. This is desirable when all hosts
are similar, such as load balancing over multiple replicas, but
undesirable when only one host out of the list is expected to
successfully connect and login. In this scenario, every other connection
attempt would hit an invalid host and fail.

The host_strategy parameter controls this host selection behavior. The
default behavior is labeled round_robin and a new strategy called
last_successful is introduced. This new strategy instructs PGBouncer
to prefer the host with most recent successful connections. When that
host fails, new connections will round-robin through the list until a
successful connection occurs.

JelteF · 2022-08-11T07:38:58Z

 	int max_db_connections = -1;
 	int dbname_ofs;
 	int pool_mode = POOL_INHERIT;
+	int host_strategy = ROUND_ROBIN;


Suggested change

int host_strategy = ROUND_ROBIN;

enum HostStrategy host_strategy = ROUND_ROBIN;

JelteF

Thank you for the contribution, it looks useful, postgres connection strings work in a similar way so it makes sense to support that as well.

I left some comments. And can you please rebase/merge this PR because there's a merge conflict.

JelteF · 2022-08-11T07:42:44Z

 			res_pool_size = atoi(val);
 		} else if (strcmp("max_db_connections", key) == 0) {
 			max_db_connections = atoi(val);
+		} else if (strcmp("host_strategy", key) == 0) {


This needs an entry in config.md to describe its usage.

Documentation is in. Feedback welcome.

JelteF · 2022-08-11T07:48:55Z

+		if (!server->pool->last_connect_failed && server->pool->db->host_strategy == LAST_SUCCESSFUL)
+			server->pool->rrcounter = server->pool->last_successful_rrcounter;


The current PR seems to have the desired effect based on the tests you wrote, but I have the feeling the same could probably be achieved without the two new fields you added: server->rrcounter and pool->last_successful_rrcounter.

How about we simply increase the pool->rr_counter whenever a connection attempt fails.

Thanks for the feedback. I simplified the logic as you suggest and it looks better to me.

dpirotte · 2022-08-29T14:07:13Z

@JelteF Touching base. Happy to incorporate any additional feedback on the new implementation.

goksgie · 2022-10-22T14:43:12Z

 	db->res_pool_size = res_pool_size;
 	db->pool_mode = pool_mode;
 	db->max_db_connections = max_db_connections;
+	db->host_strategy = host_strategy;


if the host_strategy of a database can change, should we tag it as changed?

That's a good call. Yes, I think we should tag it so behavior changes after a reload. I will make that change. Thanks for the feedback!

host_strategy is now reloadable.

show databases/show pools also now include host_strategy in their output, which makes this testable and is useful on its own.

dpirotte · 2022-11-07T15:14:18Z

@petere @JelteF I rebased this PR and made @goksgie's suggested change above.

Please let me know if anything else needs to happen to get this into a mergeable state.

JelteF · 2022-11-17T17:36:40Z

I think the code in the PR is fine now (apart from the merge conflict that occured due to another merged PR). But after playing with these changes a bit I found one other issue, and I'm wondering how useful this feature is without also addressing this issue:
server_login_retry creates a significant pause anytime a single node cannot be connected to.

@dpirotte How are you using this change, that it is useful in its current form?

JelteF · 2023-06-05T15:43:57Z

Hi @dpirotte, github notified me that you rebased this on a branch of your fork. I think a rebased version of this can be merged pretty easily. Could you still explain how you use this in practice? Because when I played with it, another bug in pgbouncer was causing serious issues for me.

dpirotte · 2023-06-05T17:44:06Z

Hi @JelteF. Yeah, I’m working on rebasing this and another patch on top of 1.19. The code works but I haven’t updated to your new (much improved!) test framework.

The purpose here is to support faster failovers in environments where 1/N hosts in the list will be the primary and (N-1)/N hosts will be replicas and where DNS updates are potentially too slow. RDS/Aurora is one such environment, and I believe some Patroni configurations are as well.

Given the following configurations:

; pg-primary is a CNAME to whichever of pg-0 or pg-1 is the primary
current = host=pg-primary.cluster.local connect_query=‘select disconnect_self() if pg_is_in_recovery()’…
proposed = host=pg-0.cluster.local,pg-1.cluster.local host_strategy=last_successful connect_query=‘select disconnect_self() if pg_is_in_recovery()’…

After a failover, current may take some time to point to the correct new primary, i.e. due to DNS caching. To speed up failovers, proposed takes the full list of potential primaries so pgbouncer can iterate through until it finds the primary. (And, to your earlier comment, this does encourage a low value for server_login_retry.)

A key part here is the connect_query, which asserts that pgbouncer is only ever talking to a writable database. Without host_strategy, using the full list of potential primaries means that (N-1)/N new connections will fail and wait for server_login_retry before trying another host. With host_strategy=last_successful, we only fail connections until a new primary is found, and direct all new connections there until the next failure. This minimizes failed connection noise and time waiting for server_login_retry.

I have another patch that adds lightweight target_session_attrs support albeit only on PG 14+ hosts where the server informs the client of its status as a primary or standby. This replaces the connect_query above, but is not a replacement for host_strategy. One could imagine having three hosts (primary and two replicas) where you want host_strategy=last_successful for the primary db connection string but still want to load balance on the replica connection string with host_strategy=round_robin.

Another option would be to keep the round-robin behavior and make target_session_attrs loop through hosts and gracefully disconnect until it finds one matching the requested state. I prefer having both configuration options so that target_session_attrs=primary host_strategy=last_successful is opening/closing as few connections on replicas as possible.

I’ll bump once the tests are ported to pytest and we’ll go from there.

dpirotte · 2023-06-07T21:33:20Z

@JelteF Rebased host_strategy with pytests if you want to take a look while I sort out the macOS build.

JelteF · 2023-06-08T12:35:23Z

 Set the pool mode specific to this database. If not set,
 the default `pool_mode` is used.

+### host_strategy


I think it would make sense to change the name of the option to load_balance_hosts. This option got added to libpq for PG16 (by me). It seems nice to use consistently named options across projects. The last_successful option name should then be changed to disable. And lets change round_robin to round-robin, to be consistent with other arguments like verify-full and read-write.

PS. I especially think consistency is nice here, because you mentioned you also want to add target_session_attrs to PgBouncer. Since target_session_attrs and load_balance_hosts can be used nicely together, I think it's especially useful to have the same names for both Postgres and PgBouncer.

Btw, adding a random as an option would be a nice future improvement to make sure PgBouncer supports the same as libpq. But that should definitely be another follow up PR. We can merge this PR easily without the random functionality.

I think it would make sense to change the name of the option to load_balance_hosts. This option got added to libpq for PG16 (by me).

Ha, good call. I noticed that you contributed this feature in the release notes. +100 to consistent naming with libpq. I'll take care of that.

I prefer the initial naming. It seems clear. load_balance_hosts seems to be a misnomer, since no "load" and no "balancing" is involved?

I prefer the initial naming. It seems clear.

I think the initial name was a fine name too. But I think being in line with naming of libpq, jdbc and npgsql is a big advantage of the load_balance_hosts naming.

load_balance_hosts seems to be a misnomer, since no "load" and no "balancing" is involved?

The "load" is the number of connection that are made by pgbouncer to the database, and those connections are "balanced" across the different servers in a round-robin fashion (or random fashion when implemented in the future). And you can enable this balancing by using disable which will select a working host and send all traffic there.

IMHO, the old naming describes how pgbouncer chooses hosts. The new naming describes what the user will see happening to their setup by changing the setting. I think I personally like names that describe the effects instead of the method.

dpirotte · 2023-06-08T21:40:29Z

@JelteF Updated naming per discussion above.

The macOS build passes now. The mingw{32,64} builds failed a couple times but seem to pass now, so those might have been random failures. The most recent build failures look like Cirrus issues: Failed to start an instance: INTERNAL: API response null(0): An error has occurred. Failed to connect to /34.122.100.68:443.

JelteF · 2023-06-08T22:05:49Z

 hostlist2 = port=6666 host=127.0.0.1,127.0.0.1 dbname=p0 user=bouncer
+hostlist_good_first = port=6666 host=127.0.0.1,127.0.0.3 dbname=p0 user=bouncer load_balance_hosts=disable
+hostlist_bad_first = port=6666 host=unresolvable-hostname,127.0.0.1 dbname=p0 user=bouncer load_balance_hosts=disable
+load_balance_hosts_update = port=6666 host=127.0.0.1,127.0.0.3 dbname=p0 user=bouncer load_balance_hosts=disable


The load_balance_hosts_update database isn't used (afaict). Did you have a test in mind for it?

JelteF · 2023-06-14T14:23:01Z

+When a comma-separated list is specified in `host`, `load_balance_hosts` controls
+which entry is chosen for a new connection.


It would be great if this same logic would be used when receiving multiple IP adresses from DNS. The easiest way to test this is to create lines in /etc/host. Libpq its load_balance_hosts version does this too. So if we don't implement this, we should at least make it explicit in the docs that this setting currently doesn't impact DNS based load balancing (but that this might change in the future).

Yeah. I'm wondering if this feature also includes #448 or at least to have it in mind.

Interesting, I didn't see that PR before (I guess I didn't go more than 2 years into PR history when joining the project). Right now #448 is completely distinct from this PR:

This PR does not implement the disable mode for DNS load balancing. It still uses round robin even if disable is used.

This PR does not implement the random mode. PR add server_shuffle_hosts #448 implements random mode for DNS load balancing only, not for host lists (certainly because host lists were not available in PgBouncer in the time that PR was written).

Th design of the config option would allow for implementing both 1 and 2 though. I think 2 isn't required to implement for this PR. A completely missing option isn't very confusing. But I think 1 is quite confusing because the existing option does not apply to all the places that you would expect, so preferably this PR would implement 1 too. If that's hard for some reason, then I think a note in the docs is also sufficient.

Hey @JelteF,

I work with the author and he's been a bit too busy to finish responding to your feedback so I'd like to take a stab at it.

I don't quite understand 1. Maybe I don't understand pgbouncer internals well enough yet but can you give me some pointers on implementing the disable mode for DNS load balancing? Sorry for such a vague ask for clarification but I do want to complete this feature as soon as I can.

So to clarify: PgBouncer does round-robin load balancing in two ways:

If you have multiple hostnames in the host key, then it will round robin across these.

If a single DNS entry returns multiple IPs, then it load balanaces using round-robin across these IPs: https://www.pgbouncer.org/faq.html#how-to-load-balance-queries-between-several-servers

This same thing is true for libpq and load_balance_hosts can be used to control the behaviour for both of these there. The implementation of load_balance_hosts in this PR only controls the behaviour of 1, not 2. And I think it should control both.

I was mistaken. Looks like getent hosts XX returns all IPs listed in /etc/hosts for XX.

OK then the only thing that makes this hard @JelteF is my own understanding. Is this where the DNS rr needs to be disabled? https://github.com/cosgroveb/pgbouncer/blob/host-strategy-cos-rebased/src/objects.c#L1647-L1649

Okay while getent hosts XX returns all the IPs listed in /etc/hosts should I expect to see all the IPs in /etc/hosts for my database host? I put together this demonstration:

def test_load_balance_hosts_disable_with_dns(bouncer, pg): bouncer.default_db = "dns_load_balance_hosts_disable" hosts = f""" 127.0.0.53 dnsdbhost 127.0.0.54 dnsdbhost 127.0.0.55 dnsdbhost """ # I have modified utils.py to set listen_addresses='*' for this # demonstrationsbut would use pg.configure/pg.reload for a real test with bouncer.run_with_appended_etc_hosts(hosts): subprocess_result = capture( ["getent", "hosts", "dnsdbhost"], ) getent_result = subprocess_result.split("\n") assert "127.0.0.53 dnsdbhost" == getent_result[0] assert "127.0.0.54 dnsdbhost" == getent_result[1] assert "127.0.0.55 dnsdbhost" == getent_result[2] bouncer.sql(query="SELECT 1", user="bouncer", password="zzzz", dbname="dns_load_balance_hosts_disable") with bouncer.admin_runner.cur() as cur: results = cur.execute("SHOW DNS_HOSTS").fetchall() result = [r for r in results if r[0] == 'dnsdbhost'][0] assert result[2] != "127.0.0.53:0"

______________________________________ test_load_balance_hosts_disable_with_dns _______________________________________ /home/admin/code/pgbouncer/test/test_load_balance_hosts.py:70: in test_load_balance_hosts_disable_with_dns assert result[2] != "127.0.0.53:0" E AssertionError: assert '127.0.0.53:0' != '127.0.0.53:0' bouncer = <test.utils.Bouncer object at 0x7f0499fbb2e0> cur = <psycopg.Cursor [closed] [BAD] at 0x7f049b445460> getent_result = ['127.0.0.53 dnsdbhost', '127.0.0.54 dnsdbhost', '127.0.0.55 dnsdbhost', '127.0.0.53 dnsdbhost', '127.0.0.54 dnsdbhost', '127.0.0.55 dnsdbhost', ...] hosts = '\n127.0.0.53 dnsdbhost\n127.0.0.54 dnsdbhost\n127.0.0.55 dnsdbhost\n ' pg = <test.utils.Postgres object at 0x7f0499fbbac0> result = ('dnsdbhost', 14, '127.0.0.53:0') results = [('dnsdbhost', 14, '127.0.0.53:0')] subprocess_result = '127.0.0.53 dnsdbhost\n127.0.0.54 dnsdbhost\n127.0.0.55 dnsdbhost\n127.0.0.53 dnsdbhost\n127.0.0.54 dnsdbhost\n127.0.0.55 dnsdbhost\n'

Because the work on the related feature, target_session_attrs is substantially complete and in need of review I do favor merging this branch as soon as possible. As you can see, the testing situation is a bit icky at the moment for the DNS sub-feature of load_balance_hosts but with some time I can complete it. It would be super beneficial to me if we can start working on getting target_session_attrs into pgbouncer in parallel and I think it would be to other users as well.

Sounds reasonable to me.

I'll get push access to @dpirotte's fork, push up a more recent rebase, and address your docs feedback below it is recommended to set server_login_retry lower than the default

cosgroveb · 2024-09-18T12:20:08Z

@JelteF I rebased this and addressed the docs feedback above. Let me know if you run into any issues merging this!

PGBouncer's default behavior is to round-robin between comma-separated hosts on each new connection attempt. This is desirable when all hosts are similar, such as load balancing over multiple replicas, but undesirable when only one host out of the list is expected to successfully connect and login. In this scenario, every other connection attempt would hit an invalid host and fail. The `host_strategy` parameter controls this host selection behavior. The default behavior is labeled `round_robin` and a new strategy called `last_successful` is introduced. This new strategy instructs PGBouncer to prefer the host with most recent successful connections. When that host fails, new connections will round-robin through the list until a successful connection occurs.

@JelteF

In order to maintain behavior that connections always start from the left-most host in the host list, increment rrcounter either before choosing a host (in the case of last_successful) or after choosing a host (in the case of round_robin). (This incorporates PR feedback from @JelteF.)

Previously, host_strategy could only be set at pgbouncer startup, which is unintuitive and inconsistent with other per-database configuration. Also, add host_strategy to both `show databases` and `show pools` output for visibility into the current setting, and to facilitate testing that the configuration properly reloads.

Previously, the host_strategy tests used 127.0.0.2,127.0.0.1 had a "bad IP" (127.0.0.2) as the first entry to force the first backend server connection to fail and verify that `host_strategy=last_successful` directs new connections to the second "good" entry. MacOS doesn't treat 127.0.0.2 in the same manner as Linux by default, so we need a different way to fail the first host in the list. Switching to an unresolvable DNS hostname works fine on both Linux and MacOS.

I incorrectly ported these tests from bash to pytest and so they were not actually verifying that subsequent connections were reusing the last successful host in the list. The tests now verify behavior correctly.

This feature does almost the same thing as the upcoming libpq param called `load_balance_hosts`, so use that name for consistency and rename the configuration options accordingly (`last_successful` => `disable`)

Lost in translation from bash tests to pytest

JelteF

I'll need to give this another round of testing, but from what I remember the functionality worked as advertised in the past.

I left a few small comments on the documentation. Also formatting check is still failing.

cosgroveb · 2024-09-18T18:57:05Z

Thanks @JelteF. will keep an eye on CI.

Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>

cosgroveb · 2024-09-25T18:29:10Z

Hey @JelteF did you run into any trouble testing this? Just checking as I'm rebasing and prepping the target_session_attrs PR.

JelteF · 2024-10-28T23:14:51Z

Sorry for the delay here. I'll try to play around with this soonish.

Aside from that, since today there's now a discord channel to discuss PgBouncer development. It would be nice if you could join that: https://discordapp.com/channels/1258108670710124574/1300532992304742481

cosgroveb · 2024-10-31T15:05:54Z

@JelteF can you link the Discord server? I wonder if I need to join the server first. Your channel link takes me here:

cosgroveb · 2024-10-31T15:08:10Z

Aha it must be PostgreSQL Hackers https://lnkd.in/g8n3dZfx

PGBouncer's default behavior is to round-robin between comma-separated hosts on each new connection attempt. This is desirable when all hosts are similar, such as load balancing over multiple replicas, but undesirable when only one host out of the list is expected to successfully connect and login. In this scenario, every other connection attempt would hit an invalid host and fail. The `host_strategy` parameter controls this host selection behavior. The default behavior is labeled `round_robin` and a new strategy called `last_successful` is introduced. This new strategy instructs PGBouncer to prefer the host with most recent successful connections. When that host fails, new connections will round-robin through the list until a successful connection occurs. --------- Co-authored-by: Brian Cosgrove <cosgroveb@gmail.com>

This was missed in pgbouncer#736

This was missed in pgbouncer#736.

This was missed in #736.

JelteF reviewed Aug 11, 2022

View reviewed changes

dpirotte force-pushed the host-strategy branch from 6087208 to 5c93a29 Compare August 17, 2022 04:09

goksgie reviewed Oct 22, 2022

View reviewed changes

dpirotte force-pushed the host-strategy branch from b35c9a6 to 5d3104f Compare November 7, 2022 01:48

dpirotte force-pushed the host-strategy branch from 97a45d5 to e0007ab Compare June 7, 2023 16:58

JelteF reviewed Jun 8, 2023

View reviewed changes

dpirotte changed the title ~~Add host_strategy parameter to db config~~ Add load_balance_hosts parameter to db config Jun 8, 2023

JelteF reviewed Jun 8, 2023

View reviewed changes

JelteF reviewed Jun 14, 2023

View reviewed changes

Comment thread doc/config.md

cosgroveb force-pushed the host-strategy branch from 657a815 to af174ed Compare September 18, 2024 12:14

dpirotte and others added 10 commits September 18, 2024 12:21

Document new host_strategy db option

eca3e40

Rewrite host_strategy tests in pytest

1744cb1

Appease flake8 linter by offering a newline

2e4e209

Fix host_strategy tests

2f69cc8

I incorrectly ported these tests from bash to pytest and so they were not actually verifying that subsequent connections were reusing the last successful host in the list. The tests now verify behavior correctly.

Rename host_strategy => load_balance_hosts

34e6fee

This feature does almost the same thing as the upcoming libpq param called `load_balance_hosts`, so use that name for consistency and rename the configuration options accordingly (`last_successful` => `disable`)

Re-add test to verify load_balance_hosts is reloadable

5b305a3

Lost in translation from bash tests to pytest

Note recommended server_login_retry

829ff98

cosgroveb force-pushed the host-strategy branch from ff49e96 to 829ff98 Compare September 18, 2024 12:23

JelteF requested changes Sep 18, 2024

View reviewed changes

Comment thread doc/config.md

Comment thread doc/usage.md Outdated

cosgroveb and others added 6 commits September 19, 2024 13:54

Fix linting failures

79244fc

Additional linting fixes

f0ceeb2

Fix linting on include files

db75ce0

Disclaim the missing DNS load balancing in this iteration of the feature

4cefc25

Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>

Fix lingering instance of old feature name

7cc633f

Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>

Bump CI

0d2835c

cosgroveb force-pushed the host-strategy branch from 3d201c8 to 0d2835c Compare September 19, 2024 13:56

This was referenced Sep 28, 2024

Add target_session_attrs support for PG 14+ servers #1177

Closed

Add target_session_attrs support for PG 14+ servers #1179

Open

Wrap line at 80

224dcac

JelteF approved these changes Oct 31, 2024

View reviewed changes

Merge branch 'master' into host-strategy

afecadc

JelteF merged commit 6ec2c65 into pgbouncer:master Nov 2, 2024

JelteF mentioned this pull request Dec 3, 2024

add server_shuffle_hosts #448

Closed

gabriele-wolfox mentioned this pull request Jan 24, 2025

fix: add support for PgBouncer 1.24 cloudnative-pg/cloudnative-pg#6630

Merged

cosgroveb added a commit to dpirotte/pgbouncer that referenced this pull request Mar 4, 2025

Add test_load_balance_hosts.py to 'make dist'

e34235b

This was missed in pgbouncer#736

cosgroveb mentioned this pull request Mar 4, 2025

Add test_load_balance_hosts.py to 'make dist' #1281

Closed

cosgroveb added a commit to cosgroveb/pgbouncer that referenced this pull request Mar 4, 2025

Add test_load_balance_hosts.py to 'make dist'

f05f7cb

This was missed in pgbouncer#736.

cosgroveb mentioned this pull request Mar 4, 2025

Add test_load_balance_hosts.py to 'make dist' #1282

Merged

JelteF pushed a commit that referenced this pull request Mar 4, 2025

Add test_load_balance_hosts.py to 'make dist' (#1282)

d363ff9

This was missed in #736.

	int host_strategy = ROUND_ROBIN;
	enum HostStrategy host_strategy = ROUND_ROBIN;

		if (!server->pool->last_connect_failed && server->pool->db->host_strategy == LAST_SUCCESSFUL)
		server->pool->rrcounter = server->pool->last_successful_rrcounter;

		When a comma-separated list is specified in `host`, `load_balance_hosts` controls
		which entry is chosen for a new connection.

Conversation

dpirotte commented Jul 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JelteF left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JelteF Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpirotte commented Aug 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpirotte commented Nov 7, 2022

Uh oh!

JelteF commented Nov 17, 2022

Uh oh!

JelteF commented Jun 5, 2023

Uh oh!

dpirotte commented Jun 5, 2023

Uh oh!

dpirotte commented Jun 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpirotte commented Jun 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JelteF Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cosgroveb Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cosgroveb commented Sep 18, 2024

Uh oh!

JelteF left a comment

Choose a reason for hiding this comment

JelteF left a comment •

edited

Loading

JelteF Aug 11, 2022 •

edited

Loading

JelteF Jun 14, 2023 •

edited

Loading

cosgroveb Sep 17, 2024 •

edited

Loading

cosgroveb commented Sep 18, 2024 •

edited

Loading