Roadmap to 0.11

[nponeccop]

We have some 16 fixes by different people unmerged. I volunteer to merge them all into one big PR which can be easily merged.

However, I don't understand the rationale behind some of the fixes and their readiness to production. So I want the authors and @melo to express their opinions on what should be shipped in 0.11 and what should be postponed to 0.12

I propose the following 4 small changes for 0.11:

• fixed unregistering of functions in workers (patch by @kappa)
• fixed Makefile.PL to work in strict sub mode (patch by @gjtorikian)
• fixed on_fail not always firing (patch by @nponeccop)
• TravisCI support (patch by @nponeccop)

The first 2 are already on @melo branch, and 2 more are my patches (I obviously claim they are ready to prime time and important, lol).

Some changes are mere documentation changes:

• Add META.json (patch by @perlancar)
• improved build instructions (patch by @gjtorikian)
• typo fix (patch by @dsteinbrunner)

The @perlancar change I don't understand, and @gitorkian doc is misleading, as those changes are only needed when developing or installing from GitHub instead of CPAN.

Some changes are ongoing efforts to get rid of Any::Moose. I don't know if they are ready or not yet:

• Switch Any::Moose-basics to Moo by @melo
• Use Type::Tiny for types (patch by @hemmop)
• Use private writer for "rwp" attributes (patch by @hemmop)

The rest are some changes I don't fully understand (including 1 documentation change):

• Worker exception support (patch by @7ojo)
• use default gearman port by default (@plicease)
• Documented prefix on worker as public attribute (patch by @plicease)
• Begin adding support for priorities (patch by @iarna)
• branch of @saymedia (includes work by @athomason and @bbeeley)

So what do you think?

[plicease]

Use gearman port by default. The synopsis here:

https://metacpan.org/pod/AnyEvent::Gearman#SYNOPSIS

implies that you can use a explicit port (x.x.x.x:port) or implicitly without a port (x.x.x.x), presumably with the default port. This patch ( https://github.com/melo/anyevent-gearman-perl/pull/6 ) just lets you do what appears to be documented in the synopsis.

Documented prefix on worker as public attribute. The documentation for the client includes a prefix attribute here:

https://metacpan.org/pod/AnyEvent::Gearman::Client#prefix-=%3E-'Str',

A prefix allows multiple applications to use the same gearman server without having to worry about them interfering with each other. The Worker class also has this prefix attribute, but it was never documented, so I submitted a patch to document it so that it could be part of the API contract as it were. While I was testing it I noticed that it didn't actually work right so I also added a patch to fix it.

It has been a long time since I submitted those patches and I can't say that I remember all of the details. I have also switched jobs and am no longer using this module, but I feel as though the patches are still worthwhile, and will do my best to answer any questions about them either here, or with the PRs on the other repository where I submitted the patches.

If this is the official repository it would be good to update the dist meta so that metacpan points here.

[bbeeley]

The changes from @saymedia were all related to consistently picking the same gearman server for the same job, and making sure that only one job runs regardless of how many different simultaneous requests there are (coalescing). We had a high traffic site, and used this to help shield us from thundering herd problems when one of our caches expired.

It has been a long time since I worked on this, and I can't say I remember the specifics of the individual changes. However, all of our code ran for years in production with these changes, so I would say they are pretty well battle-tested.

I left the company over two years ago, and the application using this code was retired at least a year and a half ago. At the time we wrote our changes we felt they were very important, and worth being merged into the main line, had there been anyone maintaining it back then.

[nponeccop]

@bbeeley Is coalescing optional? I need it, but not all jobs are idempotent. 10 launch_missile commands are supposed to launch 10 missiles, not one.

@plicease I merged the prefix branch but not the port branch as it needs some work.

So far my https://github.com/nponeccop/anyevent-gearman-perl/tree/all-fixes branch contains:

• fixed unregistering of functions in workers (patch by @kappa)
• fixed Makefile.PL to work in strict sub mode (patch by @gjtorikian)
• fixed on_fail not always firing (patch by @nponeccop)
• AppVeyor Windows CI support (patch by @nponeccop)
• TravisCI support (patch by @nponeccop)
• Add META.json (patch by @perlancar)
• Document and fix prefix on worker (patch by @plicease)

[iarna]

As for mine: I think that was just a branch that I'd started, but never actually went anywhere with, so disregarding it is appropriate.

[nponeccop]

Now also includes:

• typo fix (patch by @dsteinbrunner)
• Use default gearman port by default (@plicease)

So the remaining unmerged fixes are:

• improved build instructions (patch by @gjtorikian) - discussion needed, as the current instruction is correct for building from CPAN
• the 3 changes for Any::Moo (the PR seem to fail the tests after automated merge)
• Worker exception support (patch by @7ojo)
• branch of @saymedia (includes work by @athomason and @bbeeley)

[7ojo]

Worker exception support is related to this https://github.com/athomason/Gearman-Spawner/pull/1 but as time has passed so much I'm not sure is that needed in any case when I looked those codes again. And haven't finalized that pull request yet.. so lets disregard it for now.

[bbeeley]

Sorry for the slow reply. I don't have a clear memory of the coalescing behavior, and tracing through the code is not something I am prepared to do at this point in time. I vaguely remember it only coalescing properly when you pass a unique parameter, but I don't remember exactly what the behavior was when you didn't pass that parameter.

Reading through the other PRs makes it sound like there was at least one memory leak fixed by these changes -- it seems like they would be good to have even if you aren't interested in coalescing.

[nponeccop]

Hi guys the PR is finally there after a year. Any thoughts?