Hey @EnricoMi - I'm really eager to get this one in! Which test are failing with this improvement? Anything I can do to help?

@gabe-lyons the first parametrized run of ElasticSearchGraphServiceTest.testAddEdge fails on GitHub but not for me locally. Looks like the first run has timing issues and subsequent runs are fine. I will add some waiting to @BeforeTest to mitigate this.

And test ElasticSearchServiceTest.testElasticSearchService still requires an extra sleep for 1s: https://github.com/linkedin/datahub/pull/3160/files#diff-92a451a02ae56ca5e1b057e46524b2dd863c4e7a79feea05971de0fb7275aa41R104-R105

@gabe-lyons looks like there is still a race condition depending on the machine the tests run on. On my local machine I never see tests failing. On GitHub, tests occasionally fail (added data not visible, removed data still there).

Maybe the syncAfterWrite needs to do an extra trick:

• write a specific piece of information to ElasticSearch, like a sync flag
• busy wait for it to appear on read (sleep 10 or 100 ms inside the loop)
• remove the flag again
• busy wait for it to disappear

Given the flag has been written after changes before syncAfterWrite has been called, we should be safe to assume those changes has also made it to the read side.

@EnricoMi that approach makes sense to me. Hopefully with that we would still be able to capture most of the performance optimizations you've gained here!

Hey @EnricoMi - this looks almost ready to merge! Can you rebase on master first? There was a small change to ES tests since this PR was opened:

https://github.com/linkedin/datahub/pull/3208/files

@gabe-lyons that PR is contained already, I have rebased this recently: https://github.com/EnricoMi/datahub/commits/branch-fix-es-graph-test-sync/metadata-io/src/test/java/com/linkedin/metadata/search/elasticsearch/ElasticSearchServiceTest.java