Multiple publishes are executed when using redundant services.

[nogumaruo]

Describe the bug
In redundant task configurations, there is no mechanism for processing tasks exclusively.

To Reproduce
Steps to reproduce the behavior:

1. Launch multiple tasks with ECS Service, etc.
2. Set up scheduled delivery for any collection.
3. Cron starts simultaneously for multiple tasks.

Expected behavior
When multiple tasks are executed simultaneously, a mechanism is needed to process tasks exclusively.

System

• Node.js version: v20.11.1
• NPM version: 10.2.4
• Strapi version: 5.12.3
• Plugin version: 2.0.0-beta.10
• Database: postgres:16.2-alpine
• Operating system: macOS Sonoma 14.5

[nogumaruo]

strapi-plugin-publisher/server/services/publication-service.js

Line 20 in 11643ee

await hooks.beforePublish({ strapi, uid, entity: publishedEntity });

By using distributed locks such as redis in the beforePublish function, mutual exclusion control becomes possible.
However, in order to skip a task, it is necessary to throw an Error, and since error logs are output every time, another mechanism is preferable.
For example, it would be good to receive a boolean return value in the beforePublish function and control whether or not to execute the task according to the result.

bad

module.exports = ({ env }) => ({
  publisher: {
    enabled: true,
    config: {
      hooks: {
        beforePublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) => {
          const lockKey = `publisher:publish:${uid}:${entity.documentId}`;
          const acquired = await redis.set(lockKey, "locked", "PX", LOCK_TTL, "NX");
          if (!acquired) {
            throw new Error(`Publish lock for doc ${entity.id} is already held, skipping.`);
          }
        },
        afterPublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) => {
          const lockKey = `publisher:publish:${uid}:${entity.documentId}`;
          await redis.del(lockKey);
        },
      },
    },
  },
});

good

module.exports = ({ env }) => ({
  publisher: {
    enabled: true,
    config: {
      hooks: {
        beforePublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) : boolean => {
          const lockKey = `publisher:publish:${uid}:${entity.documentId}`;
          const acquired = await redis.set(lockKey, "locked", "PX", LOCK_TTL, "NX");
          return acquired; // true: execute task, false: skip task.
        },
        afterPublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) => {
          const lockKey = `publisher:publish:${uid}:${entity.documentId}`;
          await redis.del(lockKey);
        },
      },
    },
  },
});

[TMSchipper]

Hi @nogumaruo, thanks reaching out and for the detailed explanation!

Returning a boolean from beforePublish instead of throwing an error is indeed a cleaner approach in clustered environments. That said, if we remove the error-based flow, it might also suppress logging in non-clustered setups, which could make issues harder to detect.

We’re considering whether to extend the existing hook or introduce a new one.
We’d like to think this through a bit more to get it right, if you have any further suggestions related to the hooks, feel free to share!

If you'd like to already create a starting point, feel free to open a PR!

Thanks for the effort, cheers!

[boazpoolman]

I guess this is somewhat related to #142

[nogumaruo]

@TMSchipper
Thank you for considering this revision.

In a redundant configuration, when scheduled publishing is executed simultaneously, the Strapi REST API returns two records with the same documentId.
After investigating why two records were returned, I created this issue because we believe that simultaneous execution of plugins is affecting the operation.

In the plugin's processing, strapi.eventHub.emit is called twice in the entry.publish event, and it seems that this processing is causing multiple records with the same documentId to be returned.

Please also consider using the document API described in the official Strapi documentation.
https://docs.strapi.io/cms/api/document-service/status#update

[mmautonewimg]

I was able to partially get around this by having 1 instance with the plugin enabled and multiple instances with it disabled. However, when the instances with the plugin disabled start, it drops all rows from the actions table. I will attempt to disable cron on the additional instances, instead of the plugin itself, and see if that works.

[TMSchipper]

Hi @mmautonewimg
I hope you’re able to find a reliable workaround for now. We do have this feature on our roadmap, and if more users report the same issue or it becomes a priority for multiple projects, we’ll certainly consider moving it up in priority.

Contributions are always welcome, if anyone is open to sponsoring or donating, that can help us justify prioritizing this sooner.
Also, feel free to open a pull request if you have a proposed fix or improvement in mind. We'd be happy to review it together!

Kind regards,

[mmautonewimg]

thanks @TMSchipper , I was able to get this working using hooks to check the env var, hope this helps others.

hooks: {
        beforePublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) => {
          const shouldPublish = env.bool('PLUGIN_PUBLISHER_ENABLED', false);
          if (!shouldPublish) {
            throw new Error(`Publishing not enabled on this instance. ${entity.id} publishing skipped.`);
          }
        },
        afterPublish: async ({ strapi, uid, entity }: { strapi: Core.Strapi; uid: string; entity: any }) => {
          strapi.log.info(`Successfully published doc id: ${entity.documentId}, post id: ${entity.id}.`);
        },
      },

[boazpoolman]

Hi everybody.

May I ask why you have cron enabled on all the instances of your Strapi application? It makes sense to me that you have once instance that runs the cron jobs, and all the others just have cron disabled. That way this whole issue is solved. Maybe I'm overlooking something so it would be great if any of you could shine some light on the topic.

Apart from that I do still think that we need to have a way to prevent the (un)publish action from happening by returning a boolean in the beforeX hooks.