Investigate CI failure (possible race condition)

[gaearon]

It looks like this:

./scripts/circleci/upload_build.sh
curl: (26) couldn't open file "build/dist/react.development.js"


./scripts/circleci/upload_build.sh returned exit code 26

Action failed: ./scripts/circleci/upload_build.sh

https://circleci.com/gh/facebook/react/7816?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

It happened two times on random commits, and I don't see a pattern. Probably caused by me changing how tasks are distributed between parallel nodes in #11633. I don't understand why upload_build.sh would try to run before a build exists though (if that's what's happening?)

[gaearon]

I think this one would require some experience with CircleCI. I don't have any pointers as to where to start: I don't understand why it's failing myself. So it's not a very beginner-friendly issue (I marked the difficulty as medium for that reason).

[gaearon]

Another example: https://circleci.com/gh/facebook/react/7827?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

[ctxhou]

@gaearon I want to work on this. After doing some research on circleci document, I didn't find the document said about parallel test and deployment sequence. I'm curious about how circleci deal with parallel test and deployment sequence. I think there are two situations:

1. After all parallel test finishes, run deployment sequence. If this is true, I think it won't cause current test error.
2. Each container runs deployment after the container's test finished. I guess it is more likely in this case. Sometimes it passed, it's because node 2 ./scripts/circleci/test_entry_point.sh already built build/dist/react.development.js file. Sometimes it failed because node 2 didn't build file yet.

So I think maybe that's because each container will run deployment after that node test finished.

If the situation is as above, I come up with two solutions.

1. If we want to keep current test flow, we can try to keep each node builds time balance.
2. Do ./scripts/circleci/test_entry_point.sh this test at first, then run other test parallel.

reference link:

• circle ci: automatic-balancing-with-manual-parallel-setup
• circle ci: deployment

[gaearon]

Only node 2 does a full build. So I don't understand how deployment on other nodes could work at all.

[gaearon]

Or do nodes share the file system?

[ctxhou]

I think nodes share the file system. Otherwise, it should fail every time.

It will be easier to debug if we can add more log, like current build time and artifacts.

[gaearon]

Would it make sense to change the upload script to only run on the second node?

[ctxhou]

Yep, I think so, because in the document I can't ensure whether deployment stage is triggered after all parallel test finished.

[gaearon]

Can we just put the deployment command right into the test command? Is there any benefit to separating them?

[ctxhou]

I think it's also workable, but may lead misunderstanding. Cause we put build process to test stage.
Or we can try to use compile stage. Run './scripts/circleci/build.sh' at compile stage, so we can promise test and deployment stages have needed file.

[gaearon]

The interesting part is our regular tests don't need compilation. If "compile" stage has to run before any other stages then we'll wait unnecessarily. I'd prefer to have a single script that does everything than to buy into CircleCI specific terminology unless it buys us something real good (e.g. parallelism was very useful). Maybe can try this: https://github.com/facebook/react/compare/master...gaearon:maybe-fix-ci?expand=1.

[ctxhou]

OK, I understand your consideration.
I think this fix will work. Just tell me if anything can help.