libgearman: opt-in ketama hash-based server selection by job unique

[p-alik]

Summary

• Fixes libgearman isn't hashing submitted jobs based on unique #64: libgearman currently submits a job to the first idle connection in gearman_client_add_server() order, ignoring the job's unique entirely, so the same unique can land on any server depending on load. libhashkit is already linked in but was never wired up for this.
• Adds an opt-in API, gearman_client_set_server_selection_by_unique() / gearman_client_server_selection_by_unique(), that routes a job submission by hashing its unique against a ketama consistent-hash ring built from the client's current connection list, with fail-over to the next server in ring order if the hash-picked one is unavailable. Disabled by default, so existing behavior is unchanged unless a caller opts in. Restricted to actual job-submission commands (SUBMIT_JOB*/SUBMIT_REDUCE_JOB*); GET_STATUS and friends are untouched, and an empty unique falls back to the default selection.
• gearman_client_clone() now copies this option like the client's other persistent settings (it was silently dropped by clone before).
• New Sphinx doc page (gearman_client_set_server_selection_by_unique.rst), linked from the libgearman index, whose intro previously only described the old order-based routing.

Test plan

• libgearman/server_selection.cc's ketama ring/selection covered directly in tests/libgearman-1.0/internals.cc (determinism, busy-connection skipping, ring-order fail-over, empty connection list).
• API-level unit tests in tests/libgearman-1.0/client_test.cc (getter/setter contract, NULL-safety, survives gearman_client_clone()).
• Integration tests in tests/libgearman-1.0/multi_client_test.cc against two real gearmand instances: same unique routes to the same server across independent clients (proven via identical job_handle from server-side coalescing), submission with an empty unique falls back to default selection, and submission still succeeds by failing over to the surviving server when the hash-picked one is down.
• Full existing suite (t/client, t/internals, t/multi_client) passes, and the docs build cleanly under sphinx-build -W.

🤖 Generated with Claude Code

[esabol]

There's a lot to digest here. I'll get back to this soon.

[esabol]

So will clients like the PHP extension and the Gearman::Client Perl module need to be enhanced to support this feature?

[p-alik]

Yes, for the language bindings that actually wrap libgearman — but it depends on which one:

• PHP (pecl/gearman): it's a C extension binding directly to libgearman's gearman_client_st/gearman_worker_st API, so GearmanClient would need a new method added (e.g. setServerSelectionByUnique()) to expose this at the PHP level. Nothing breaks without it — the option defaults to off — but PHP users can't opt in until that binding exists.
• Gearman::XS (the XS-based Perl bindings) would be in the same position — it also binds to libgearman via C, so it'd need a matching accessor added.
• Gearman::Client (the plain Gearman CPAN distribution) is a different story: that one is a pure-Perl implementation that doesn't link against or call libgearman at all. It wouldn't gain anything from this change automatically — routing by unique would have to be reimplemented independently in Perl if wanted there.

So: PHP and Gearman::XS need a thin wrapper addition each; the pure-Perl Gearman::Client is untouched by this and out of scope for a libgearman-side PR.

[esabol]

To be honest, this doesn't seem very useful to me without downstream support from the PHP, Perl, Python, et al. client/worker APIs. I guess we could just hope for the best and have a "build it and they will come" mindset, but it took almost a decade to get PHP to support SSL.

I don't suppose we could just make this the default behavior when there's more than one server? What's the downside? Would it break any existing gearmand usage?

[esabol]

This PR now has conflicts which need to be resolved.

[esabol]

Did you see my post here, @p-alik ?

[p-alik]

I don't suppose we could just make this the default behavior when there's more than one server? What's the downside? Would it break any existing gearmand usage?

I'd keep it opt-in, at least for now. Several existing setups would break if it became the default:

• Primary/standby server lists. Today libgearman sends each job to the first idle connection in gearman_client_add_server() order, so in practice "server 1, then fail over." Anyone who lists a primary and a standby would suddenly get half their jobs on the standby.
• Workers attached to only some servers. If the workers for a function connect only to the first server, that works today. With hashing, jobs routed to the other servers would sit there and never run.
• Load and connection pattern. Every client would end up connected to every server rather than mostly one. That changes queue sizes, monitoring and persistent-queue load per server.
• Implicit behavior change. Adding a second server to an existing config would silently change where jobs go.

On the bindings concern, I agree a new setter on its own doesn't reach PHP/Perl/Python users. I'd like to also expose it as a client option flag, GEARMAN_CLIENT_SERVER_SELECTION_BY_UNIQUE (next free bit, 1 << 11), handled in gearman_client_add_options(), gearman_client_remove_options() and gearman_client_has_option(). It also needs adding to the usable_options[] list in gearman_client_set_options(), which otherwise ignores unknown bits.

Bindings that pass option flags through can then use it without a new release. I checked pecl-gearman: GearmanClient::addOptions(int) hands the int straight to gearman_client_add_options() without masking it. So with current pecl, PHP users could opt in with

$client->addOptions(2048); // GEARMAN_CLIENT_SERVER_SELECTION_BY_UNIQUE

and a named constant can follow in pecl whenever convenient. (addOptions() rather than setOptions(), since pecl enables GEARMAN_CLIENT_FREE_TASKS itself and setOptions() would clear it.)

As for why it's useful even before every binding catches up: a job's unique is only deduplicated within a single server. Without consistent routing, two submits with the same unique can land on different servers and both run. That's what #64 is about.

If it proves itself, flipping the default in a later major release with a changelog note seems reasonable to me.

[esabol]

I'd like to also expose it as a client option flag, GEARMAN_CLIENT_SERVER_SELECTION_BY_UNIQUE (next free bit, 1 << 11), handled in gearman_client_add_options(), gearman_client_remove_options() and gearman_client_has_option(). It also needs adding to the usable_options[] list in gearman_client_set_options(), which otherwise ignores unknown bits.

That sounds like a good idea to me. That's something you would need to add to this PR, right? If so, please do when you have the time. And additional test(s) of this new client option, please, if appropriate.

I was going to make the next version of gearmand version 2.1.1, but with this new feature, version 2.2.0 would be more appropriate, I think. Do you agree, @p-alik?

[p-alik]

I was going to make the next version of gearmand version 2.1.1, but with this new feature, version 2.2.0 would be more appropriate, I think. Do you agree, @p-alik?

Yes, I agree with 2.2.0. The PR adds public API (the setter/getter and a new gearman_client_options_t flag) without changing existing behavior, so a minor bump fits.

The flag and tests are pushed:

• 9fc6238 adds GEARMAN_CLIENT_SERVER_SELECTION_BY_UNIQUE (1 << 11). It's handled in gearman_client_add_options(), remove_options(), has_option(), gearman_client_options() and usable_options[] in gearman_client_set_options(), and it controls the same setting as gearman_client_set_server_selection_by_unique(). The docs are updated, and a new test covers add/remove/has/set, how the flag and setter interact, other flags staying untouched, and gearman_client_clone(). With current pecl-gearman, $client->addOptions(2048) turns it on.
• 0e78ac1 makes the routing test in multi_client_test.cc stronger. The old version passed even with hashing off, because both clients went to the first server anyway and the duplicate unique was merged there. It now submits 32 uniques, asks each server which jobs it holds, and requires both servers to have received some. With hashing disabled it fails with "all 32 jobs landed on one server".

Since 2.2.0 adds library functions, should GEARMAN_LIBRARY_VERSION in configure.ac also go from 8:0:0 to 9:0:1? It hasn't changed since 2012, so I'm not sure whether the project still maintains it. Happy either way.

[esabol]

Since 2.2.0 adds library functions, should GEARMAN_LIBRARY_VERSION in configure.ac also go from 8:0:0 to 9:0:1? It hasn't changed since 2012, so I'm not sure whether the project still maintains it.

it probably should increase since there are new library functions. Why 9:0:1 though?

[p-alik]

Why 9:0:1 though?

It comes from libtool's version-info update rules for current:revision:age:

1. If the library source changed, increment revision.
2. If any interfaces were added, removed or changed, increment current and set revision to 0.
3. If interfaces were added, increment age.
4. If interfaces were removed or changed incompatibly, set age to 0.

From 8:0:0, step 2 gives 9:0:0 and step 3 gives 9:0:1. On Linux the soname is libgearman.so.(current - age), so it stays libgearman.so.8 and existing binaries keep working without a rebuild. Only the file name changes, from libgearman.so.8.0.0 to libgearman.so.8.1.0.

8:0:0 was set for 1.1.0 (2012) and never bumped since, so I compared the public API of 1.1.0 with this branch to make sure step 4 doesn't apply:

• No GEARMAN_API function was removed, and the 164 functions present in both have identical signatures.
• 11 functions were added: gearman_client_cancel_job, gearman_client_has_active_tasks, gearman_client_set_ssl, gearman_worker_set_ssl, gearman_job_error, gearman_job_use_client, gearman_task_exception, gearman_task_has_exception, gearman_task_is_finished, and the two from this PR.
• struct gearman_task_st lost bool is_initialized (ab5cedf), but its size and the offset of _impl are unchanged, because _impl stays pointer-aligned.
• Only the *_MAX end-of-list values in the option and return enums changed, and those aren't meant to be passed as values.

So everything since 1.1.0 has been additive, and 9:0:1 matches what the library actually is. A single bump now also covers the additions that missed one. This is a header and symbol comparison, not an abi-compliance-checker run.

Should I add the bump to this PR, or would you rather do it in the 2.2.0 release commit?

[esabol]

Should I add the bump to this PR, or would you rather do it in the 2.2.0 release commit?

Please do it here in this PR. I'd probably just mess it up. 😄

Thanks for the explanation! I never knew about "age"...

[p-alik]

Please do it here in this PR.

Done in ab7f394: GEARMAN_LIBRARY_VERSION is now 9:0:1. I also updated support/gearmand.spec.in, which listed libgearman.so.8.0.0 by name; it's now libgearman.so.8.1.0.

9:0:1 doesn't produce libgearman.so.9.0.1, because current:revision:age is libtool's input, not a version number. On Linux libtool maps it to:

libgearman.so.(current - age).(age).(revision)

So 9:0:1 gives libgearman.so.8.1.0, and the old 8:0:0 gave libgearman.so.8.0.0. The first number, current - age, is also the soname. It stays libgearman.so.8, so programs linked against the old library load the new one without a rebuild. age = 1 records that the library still provides everything the previous interface version did.

A libgearman.so.9 would only be correct if something had been removed or changed incompatibly. Then every program would need relinking, which this release doesn't require. The same reasoning is why libtool's manual warns against setting -version-info to the release number, e.g. 2:2:0 for 2.2.0. A local build produces libgearman.so.8.1.0 with soname libgearman.so.8, as expected.