Get rid of consuming events from RabbitMQ

[vikasrohit]

Currently we might be using few RabbitMQ event to complete some of the use cases and features in project service. There was one major part which was dependent on the RabbitMQ events and it was tc-connect2sf which is recently moved away from RabbitMQ to Kafka, so we have to start working on moving all the logic which is still using RabbitMQ, to use Kafka instead.

fyi @maxceem as we discussed.

[maxceem]

I've noticed one side effect after moving the processing phase created event from RabbitMQ to Kafka.
When we add a new phase in Connect, we reload the list of phases immediately after that. Phases are reloaded from ES so at the time we reload phases it often happens that the newly added phase is not yet indexed to ES so we cannot see a newly added phase.

It seems that RabbitMQ processed the events faster so this issue didn't happen before. Note, that we index phases inside Project Service for now because of cascading updates, so before indexing was handled by RabbitMQ event, and now by Kafka event.

I would think what would be the best solution here, for now just let you know.

[vikasrohit]

Good observations @maxceem and good to know that fact that RabbitMQ was faster then Kafka at least for our use case. And I think one important reason of delay in kafka based processing is that we are using REST API endpoint to post a message to bus which definitely causes full HTTP request cycle latency before actual message reaches to the Kafka server. I am not sure if after reaching kafka server, message is delayed further or not.
May be, we can add false loading of 5 seconds or so, just like we did for invite accept in Connect.

that we index phases inside Project Service for now because of cascading updates

Just for reference, can you please mention what type of cascading we are doing with phases? I mean I remember cascading for milestones because we don't allow setting of their start dates and we were controlling the start-end dates of milestones automatically. But I am unable to recall why what cascading updates we need for phases.

[maxceem]

@vikasrohit

1. Just for reference, can you please mention what type of cascading we are doing with phases? I mean I remember cascading for milestones because we don't allow setting of their start dates and we were controlling the start-end dates of milestones automatically. But I am unable to recall why what cascading updates we need for phases.

   We do cascade updates for the order field, see:

   • add https://github.com/topcoder-platform/tc-project-service/blob/develop/src/routes/phases/create.js#L76-L92
   • update https://github.com/topcoder-platform/tc-project-service/blob/develop/src/routes/phases/update.js#L89-L151

   Though I can see that we never use order in Connect App and more than that in real data we never define it at all:

   

   So I would suggest removing this logic from the backend.

2. As we don't really use cascade updates, so to solve this issue in Connect App we can just add the newly created phase client-side without reloading all the phases.

[maxceem]

This is done via 3 PRs:

• Project Service feat: remove RabbitMQ and use Kafka #585
  • move actual handles from RabbitMQ to Kafka
  • removed unused RabbitMQ handles code
  • removed all RabbitMQ events firing
  • added support for graceful shutdown of index-kafka.js (otherwise it may bring issues even in production, as badly stopped service might still listen to events in the background and cause issue like versions conflict during ES indexing)
  • removed RabbitMQ from production and local deployment
  • fix unit tests to use a single ES service which refreshes data after updating
  • removed redundant seedElasticsearchIndex.js script (we have a new one "npm run data:import")
• Connect App feat: don't reload phases after adding appirio-tech/connect-app#4155
  • fix for the issue during phases adding described above - we don't reload phases and just add a new phase client-side
• Project ES Processor feat: enable phase create/update handlers project-processor-es#50
  • Project Service doesn't handle phases reindexing anymore, so this is enabled in Project ES Processor

[vikasrohit]

So I would suggest removing this logic from the backend.

@maxceem the PR has this logic removed now, right?

[maxceem]

@maxceem the PR has this logic removed now, right?

@vikasrohit No, I didn't remove it without the confirmation. But I can do that.

[vikasrohit]

okay. Does that mean the PR is still updating the order field?

[maxceem]

Yes, potentially it can update order, but in reality, it never happens, because we never set order. To trigger the cascading logic we have to set order first.

[vikasrohit]

okay, please remove the logic for handling order as we are not using it as of now and it is causing more problems than future features.

[maxceem]

@vikasrohit removed phase order cascading updates and added one more commit c5a01ed to the PR.