Reimplement the legacy processing logic with APIs

[vikasrohit]

The current version is using SQL scripts to perform the direct project CRUD operations which makes hte logic too complicated and one more place for Informix connection overhead. So, we should avoid creating database connections from this processor. We should:

1. Remove all the SQL calling for interacting with direct and use direct's APIs which we were using in v4 projects api
2. Also when updating the directProjectId back into the Connect projects database, we should not use SQL script again and rather call the projects api to update the project data. That would automatically index the project in ES which is missing as of current implementation.

[vikasrohit]

@maxceem we need to wrap this asap after other P0 fixes we are doing in the 2.5. release. We can live with 1 for some time but we need to wrap 2 as soon as possible.

[maxceem]

@vikasrohit I can take care fast about the urgent part 2 via F2F/PrivateTask. And after run a challenge to cover part 1. It looks like most of the code should be rewritten for this.

[vikasrohit]

Okay. Lets hold on the part 1 for now, let me clarify this with the broader team, if that was done with some specific intent. But part 2 is definitely urgent for us.

[maxceem]

The F2F challenge for part 2 is on http://www.topcoder.com/challenges/30111432

[maxceem]

@vikasrohit the second part is generally done in F2F, I'm only waiting for unit tests to be fixed in the submission. Though during testing, I've noticed 2 issues: one easy to fix and another one could be hard.

1. When we call endpoint PATCH /v5/projects/{id} to set directProjectId with use M2M token. In the PATCH endpoint, we are also updating updatedBy property with the userId of who did such an update. For DEV M2M userId is -101 which doesn't pass userId validation. For such case I guess we can skip updating updatedBy if userId is not a positive number, something like this:

   

2. Another issue is more complex.
   It happens sometimes that inside project-processor-es message project.action.create is processed after message project.action.update for a newly created project. So the workflow looks like this (sometimes):

   • create a new project using Projects Service
   • Projects Service sends message project.action.create to Kafka via Bus API
   • legacy-project-processor handles project.action.create and calls PATCH /v5/projects/{id} of Project Service
   • after update Project Service sends message project.action.update to Kafka via Bus API
   • project-processor-es handles message project.action.update first which causes an error as project is not yet exists in ES
   • project-processor-es handles message project.action.create after update and it issuccessful.
   • as a result we don't have directProjectId added in ES

   The most easier way to reproduce this issue is to send several requests to Project Service API to create new projects. For example 2 requests for creating projects one by one. I can see that in Bus API messages are send in the correct order:

   

   But in project-processor-es the order is incorrect:

   • create project 22 - this is right:

     

   • update project 22 (with directProjectId) - this is right:

     

   • update project 23 (with directProjectId) - this is incorrect and causes an error:

     

   • create project 23:

     

   REASON
   The reason for this issue is that Kafka doesn't guarantee the order of messages in different topics, and in our case, we do have different topics: project.action.create and project.action.update. Actually, Kafka even doesn't guarantee the order of message even for a single topic if such a topic has more than 1 partition. So Kafka only guarantees the order of messages inside 1 partition.

   This is a very general issue, and I think we might come across it in various situations in the future. Though if we have the only one partition per topic than we should be quite safe in most of the situations. But we have to keep it in mind, especially when we would remove RabbitMQ in favor of Kafka - we should avoid immediate update operations after create.

   As a quick solution for the current issue, we have now, I would suggest adding a delay inside legacy-project-processor before sending the request to update the project about 3 - 5 seconds. This should reduce the frequency of such issues even though not with 100% chance. And we could control the consistency with our scheduled script we are working on now and see if this issue happens often or no.

[vikasrohit]

for 1, @maxceem, I have intentionally set the -101 for patch call to allow update projects via m2m calls and I made this change for updating a project's status from tc-connect2sf and it seems to be working fine for me. If it is throwing validation error, it must be the same on production as well for tc-connect2sf (I need to check). But in general it should not happen as we recently got our tc-core-lib updated to support having a calculated value instead of -101 for m2m calls so that we can identify which m2m client updated the project last. The new logic also generates a negative number as azp hash to avoid collisions with real user ids. So, we should be good with negative user ids for such m2m calls, and if we need to run a SQL script to fix this, we can do that.

for 2, I think the delay logic is fine for now but we have to think of a better solution. What is the standard practice around that? As it seems a general issue so should be a fix. Can we NACK a message in Kafka? I mean, if we see project.action.update for a project which does not yet exists in ES, can we NACK to the server, like we do in RabbitMQ?

[maxceem]

If it is throwing validation error, it must be the same on production as well for tc-connect2sf (I need to check).

The validation error now comes from both new processors: project-processor-es and legacy-project-processor. They both expect updatedAt to be a positive number as any other id https://github.com/topcoder-platform/project-processor-es/blob/develop/src/services/ProcessorServiceProject.js#L38 and https://github.com/topcoder-platform/legacy-project-processor/blob/develop/src/services/ProcessorService.js#L573.
We can fix that. Though I just would like to confirm that we really want to update updatedAt via m2m userId.

for 2, I think the delay logic is fine for now but we have to think of a better solution. What is the standard practice around that?

I didn't come across such a situation myself. Quick googleing tells that the way to go is to create some kind of buffer in the listener service which wouldn't let the messages to be processed immediately on arrival, but wait for a little, sort the messages which are currently in the buffer and only after pop the earliest message. Though, I'm not sure how long messages should stay in such a buffer for our real-life cases.

Maybe there are other solutions, and we can ask about best practices other people on Topcoder. In case they have already solved it to some other of Topcoder services. Or we can run an Indentation challenge and ask the community for help.

As it seems a general issue so should be a fix. Can we NACK a message in Kafka?

As much as I understand in Kafka we cannon NACK individual messages, as we only control which messages have been processed by the offset. So we only can point until which message have we processed the topic. Not sure if it could help us for our case.

[maxceem]

@vikasrohit the part 2 is done via PR #4 to dev.

Let me know if also would like me to create a hotfix against PROD as well.

Also, for this fix to work properly, another hotfix should be applied first to project-processor-es:
DEV topcoder-platform/project-processor-es#12
PROD topcoder-platform/project-processor-es#13
Note, that this hotifx to project-processor-es also, has to be applied so tc-connect2sf works properly or changes done by that service in projects would not be reindexed.

[vikasrohit]

1. Yes, we do want to store the m2m user id (which is now derived via hash of m2m client id as a negative integer) in updatedAt field because otherwise we don't have any way to track who updated the data via the m2m calls.
2. Yep, I would ask the team when they are back from holidays. Let me know if you think we can get some answer by community meanwhile by any means.

[vikasrohit]

@maxceem I would merge the dev versions of the hotfix first to test it out. I think we can wait till Thursday to apply the hotfix to the production as tomorrow is holiday and I don't want to do a prod deploy before holiday.

[maxceem]

After deploying to DEV, the fix for the second part looks good and Direct Project is created and indexed in ES https://connect.topcoder-dev.com/projects/8620.

I can see that we get directProjectId on the listing page which for sure gets data from ES:

[maxceem]

This fix also requires to apply another fix to Connect appirio-tech/connect-app#3471 so it doesn't request user details about M2M users.