Sidekiq 5 UI compatibility #458

ananthakumaran · 2021-10-10T07:15:02Z

Adds support for sidekiq 5 UI. Threads (aka concurrency) is set to 0 since we allow custom dequeuers which might allow jobs based on different constrain. Busy count should be accurate. The Quiet button would cause the node to unsubscribe from all queues.

Code

This change introduces the concept of worker nodes. The list of active worker nodes is tracked in a set exq:processes. For each worker node, some metadata like subscribed queues, number of busy jobs, start time etc are tracked. A new GenServer called Exq.Node.Server is introduced to send heartbeat and the metadata info to the redis every 5 seconds. It also cleans up the dead worker nodes' info from redis.
The concept of per worker signal is introduced. Currently we support TSTP (quiet), which will ask the worker node to unsubscribe all active queue subscription.
Per worker metadata is stored under exq:{node_id} hash and kept updated by the GenServer. This info will expire in 60 seconds (which should happen when the worker dies)
Per worker active jobs are stored in exq:{node_id}:workers hash. Each individual job will get removed once it's done. The whole hash is deleted on node startup and on dead node cleanup

Upgrade Instructions

There are 2 types of breaking changes introduced by this PR.

We used to store the set of actives jobs under exq:processes, but the list of active worker nodes node_id is being stored now. The new version gracefully handles unknown entries (it will be cleaned up eventually by prune_dead_nodes). So upgrade should not be an issue. But the old code won't handle node_id (non json), so it can't be rolled back without first deleting the exq:processes set manually from redis console.
Some of the data structures used also got changed which will break ExqUI. Specially Exq.Api.processes/1 will return struct with different attributes. Those who use ExqUI should plan the upgrade of both Exq and ExqUI. The next release of ExqUI will be compatible with the new struct.

screenshot from single node

screenshot from multiple nodes (using custom identifier functionality)

fixes #439 #289 #288

The old version used to store job details under processes key. The periodic prune will remove those jobs as well any dead nodes from the processes set

akira · 2021-10-25T16:10:40Z

Are these changes compatible with existing ExqUI or is the idea to release it together with new UI rewrite?

ananthakumaran · 2021-10-25T16:18:53Z

Are these changes compatible with existing ExqUI or is the idea to release it together with new UI rewrite?

I haven't tested it, but I guess it would be broken because the Process struct got updated. Yes, my plan is to make a exq release followed by the new exq_ui release.

akira · 2021-11-03T16:00:44Z

lib/exq/node/server.ex

+      JobStat.node_ping(redis, namespace, node)
+      |> process_signal(state)
+
+    if Integer.mod(state.ping_count, 10) == 0 do


What is 10 for? Should be configured elsewhere or constant?

We have to remove the dead nodes from the processes set, the mod 10 is just a optimization to avoid doing the cleanup check on each ping instead do it on every 10th ping. The number 10 itself comes from sidekiq implementation. If you feel this should be configurable, I can move it to application config

akira · 2021-11-03T16:09:23Z

Possible to add more description of changes in PR / commit such as adding new GenServer for Node, changes to JobStat, etc.

akira · 2021-11-03T16:10:51Z

lib/exq/node/server.ex

+
+  defp process_signal(nil, _), do: :ok
+
+  defp process_signal("TSTP", state) do


What is this one for? I didn't see it used elsewhere..

The Quiet button in the UI pauses the worker (the worker stops picking up any new job). This is done via a signal list. UI will push TSTP string into the list and the worker will pop that and perform the action. I plan to add same feature in the Exq UI later as well.

akira · 2021-11-03T16:23:42Z

Anything special instructions needed when upgrading to this branch once it lands? For handling existing inflight jobs / working nodes, job stats, etc..

ananthakumaran · 2021-11-04T02:29:22Z

Anything special instructions needed when upgrading to this branch once it lands? For handling existing inflight jobs / working nodes, job stats, etc..

If ExqUI is not used, then nothing needs to be done. There are some breaking changes related to where which info is stored, but the prune_dead_nodes parts will take care of the migration (it will remove the old job entries from exq:processes set). Though, the upgrade can't be rolled back easily. As the old code will expect jobs in exq:processes set. To rollback the exq:processes needs to be deleted (we can include this instruction as part of release info)

If ExqUI is used, then the user has to wait for the new ExqUI release and upgrade ExqUI after Exq upgrade.

ananthakumaran · 2021-11-04T02:57:20Z

Possible to add more description of changes in PR / commit such as adding new GenServer for Node, changes to JobStat, etc.

I have updated the PR description with more info. Let me know if anything else needs to be covered

akira · 2021-11-12T06:57:29Z

Thanks for the updates! Looks good I think - is the other PR also ready in ExqUI or still pending work on it?

ananthakumaran · 2021-11-12T12:15:35Z

Thanks for the updates! Looks good I think - is the other PR also ready in ExqUI or still pending work on it?

akira/exq_ui#102 is ready for review. It includes the busy tab and related changes introduced in this branch.

akira · 2021-11-15T18:19:51Z

@ananthakumaran Great thanks, will take a look at the other one as well, sorry for delay

akira · 2021-11-28T02:26:37Z

Thanks for the updates! Looks good I think - is the other PR also ready in ExqUI or still pending work on it?

akira/exq_ui#102 is ready for review. It includes the busy tab and related changes introduced in this branch.

Sounds good, I tried it out and works well! 🎉 Let me know your preference for release timing on these. For the exq_ui maybe some additional unit tests (can be separate PR also).

ananthakumaran · 2021-11-28T03:44:01Z

Both PRs can be merged to master. As for release, we could take a bit more time. I will open separate PRs for CI setup and tests for exq_ui. We will also try to get this tested out in our prod service (which is getting delayed due to phoenix version dependency)

akira · 2021-11-28T03:58:03Z

Both PRs can be merged to master. As for release, we could take a bit more time. I will open separate PRs for CI setup and tests for exq_ui. We will also try to get this tested out in our prod service (which is getting delayed due to phoenix version dependency)

Sounds good! I added as maintainer to the other project as well, please feel free to merge.

akira · 2021-12-12T21:30:39Z

This has been released, Exq 0.16.0 and ExqUI 0.1.0 🎉

ananthakumaran added 4 commits October 10, 2021 09:37

add support for sidekiq 5 stats

0add05b

expose api to get list of active nodes

eee55ce

System.pid/0 is only available from 1.9

b2c50e2

prune dead node ids

7cbe7fe

The old version used to store job details under processes key. The periodic prune will remove those jobs as well any dead nodes from the processes set

ananthakumaran marked this pull request as ready for review October 15, 2021 08:48

remove the busy job details of dead nodes

16e707d

akira reviewed Nov 3, 2021

View reviewed changes

ananthakumaran merged commit 23e3de6 into master Nov 28, 2021

ananthakumaran mentioned this pull request Dec 4, 2021

Not showing up in Sidekiq Web UI #222

Closed

akira mentioned this pull request Dec 12, 2021

Replace deprecated Supervisor calls #453

Merged

akira mentioned this pull request Dec 12, 2021

Update README: use latest Phoenix child spec style #459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sidekiq 5 UI compatibility #458

Sidekiq 5 UI compatibility #458

ananthakumaran commented Oct 10, 2021 •

edited

akira commented Oct 25, 2021

ananthakumaran commented Oct 25, 2021

akira Nov 3, 2021

ananthakumaran Nov 4, 2021

akira commented Nov 3, 2021

akira Nov 3, 2021

ananthakumaran Nov 4, 2021

akira commented Nov 3, 2021

ananthakumaran commented Nov 4, 2021 •

edited

ananthakumaran commented Nov 4, 2021

akira commented Nov 12, 2021

ananthakumaran commented Nov 12, 2021

akira commented Nov 15, 2021

akira commented Nov 28, 2021

ananthakumaran commented Nov 28, 2021

akira commented Nov 28, 2021

akira commented Dec 12, 2021


		defp process_signal(nil, _), do: :ok

		defp process_signal("TSTP", state) do

Sidekiq 5 UI compatibility #458

Sidekiq 5 UI compatibility #458

Conversation

ananthakumaran commented Oct 10, 2021 • edited

Code

Upgrade Instructions

akira commented Oct 25, 2021

ananthakumaran commented Oct 25, 2021

akira Nov 3, 2021

Choose a reason for hiding this comment

ananthakumaran Nov 4, 2021

Choose a reason for hiding this comment

akira commented Nov 3, 2021

akira Nov 3, 2021

Choose a reason for hiding this comment

ananthakumaran Nov 4, 2021

Choose a reason for hiding this comment

akira commented Nov 3, 2021

ananthakumaran commented Nov 4, 2021 • edited

ananthakumaran commented Nov 4, 2021

akira commented Nov 12, 2021

ananthakumaran commented Nov 12, 2021

akira commented Nov 15, 2021

akira commented Nov 28, 2021

ananthakumaran commented Nov 28, 2021

akira commented Nov 28, 2021

akira commented Dec 12, 2021

ananthakumaran commented Oct 10, 2021 •

edited

ananthakumaran commented Nov 4, 2021 •

edited