beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aljoscha <...@git.apache.org>
Subject [GitHub] incubator-beam pull request #409: [BEAM-321] Fix Flink Comparators
Date Wed, 01 Jun 2016 16:15:54 GMT
GitHub user aljoscha opened a pull request:

    https://github.com/apache/incubator-beam/pull/409

    [BEAM-321] Fix Flink Comparators

    `KvCoderComparator` and `CoderComparator` were hashing the key directly
    while doing comparisons on the encoded form. This lead to
    inconsistencies in GroupByKey results with large numbers of elements per
    key.
    
    This changes the comparators to hash on the encoded form and also adds a
    test case to verify correct behavior. 
    
    I made the test a `RunnableOnService` test that verifies that runners use the encoded
form of a value for hashing/comparison. In the test, I use a bogus key that returns garbage
from `equals()` and `hashCode()`. The result of the test is correct if only the encoded form
is used for hashing/comparison.
    
    R: @tgroh for the test, do you think there's a better way of doing it?
    CC: @tgroh the newly introduced test hangs on the `InProcessPipelineRunner`, see `InProcessGroupByKeyTest`
that I added for quickly checking this
    CC: @francesperry is this new test in line with the Beam model

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aljoscha/incubator-beam flink/fix-comparator

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-beam/pull/409.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #409
    
----
commit 8b269b542032c887ea149c50245de8d07a94fd51
Author: Aljoscha Krettek <aljoscha.krettek@gmail.com>
Date:   2016-06-01T09:56:18Z

    [BEAM-321] Fix Flink Comparators
    
    KvCoderComparator and CoderComparator were hashing the key directly
    while doing comparisons on the encoded form. This lead to
    inconsistencies in GroupByKey results with large numbers of elements per
    key.
    
    This changes the comparators to hash on the encoded form and also adds a
    test case to verify correct behavior.

commit 6866fd740af07a214ef5a9be39983260878ecb8d
Author: Aljoscha Krettek <aljoscha.krettek@gmail.com>
Date:   2016-06-01T09:58:48Z

    Fix typo in KvCoderComperator, is now KvCoderComparator

commit 6334b87ef3d2a90c540cf89aadab013dcded1483
Author: Aljoscha Krettek <aljoscha.krettek@gmail.com>
Date:   2016-06-01T15:54:37Z

    move to RunnableOnService test

commit 85defbc67323e10a87835d2514f45ad238e47842
Author: Aljoscha Krettek <aljoscha.krettek@gmail.com>
Date:   2016-06-01T16:14:51Z

    make test class package private

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message