[BEAM-5184] Multimap side inputs with duplicate keys and values are being lost

[VaclavPlajt]

BEAM-5184 Side inputs aggregating multimap changed from HashMultimap to ArrayListMultimap, which allows for duplicated key-value items. Tests in ViewTest modified to exercise duplicate key-value items scenario.

---

Follow this checklist to help us incorporate your contribution quickly and easily:

• Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Flink	Gearpump	Samza	Spark
Go		---	---	---	---	---	---
Java
Python		---		---	---	---	---

[timrobertson100]

Retest this please

[VaclavPlajt]

Retest this please

[lukecwik]

Run Java PreCommit

[lukecwik]

Run Dataflow ValidatesRunner

[lukecwik]

Run Java PreCommit

[lukecwik]

Run Java PreCommit

[VaclavPlajt]

It seems that Google Cloud Dataflow Runner did not pass one of the updated tests. I tried with direct runner and it pass. Can somebody check the test?

[lukecwik]

I'll try to look at the failure in Dataflow, it's side input handling is different.

The Jenkins logs for org.apache.beam.sdk.transforms.ViewTest.testMultimapSideInputWithNonDeterministicKeyCoder fail with

Expected: iterable over [<KV{apple, 1}>, <KV{apple, 1}>, <KV{apple, 2}>, <KV{banana, 3}>, <KV{blackberry, 3}>] in any order
     but: No item matches: <KV{apple, 1}> in [<KV{apple, 2}>, <KV{apple, 1}>, <KV{blackberry, 3}>, <KV{banana, 3}>]

The error message is implying a comparison issue since all the values do exist in the actual output

[VaclavPlajt]

I would say that the error message is a bit misleading. It should say that there is no second KV{apple, 1} in presented collection. The message is hard to understand.

[lukecwik]

Yes your right. What I find odd is that only the one failed.

[lukecwik]

I figured out that the issue is that BatchViewOverrides.java is also using a HashMultimap instead of a list based multimap like ArrayListMultimap. I'll send you a patch to add to your PR.

[lukecwik]

Opened up seznam#25 against your PR branch.

[VaclavPlajt]

Run Dataflow ValidatesRunner

[VaclavPlajt]

Run Java PreCommit

[VaclavPlajt]

Run Java PreCommit

[lukecwik]

Thanks for your contribution. This should make it out into the 2.7.0 release which is starting soon.

[VaclavPlajt]

Great. 👍