[GitHub] flink issue #4748: [hotfix][tests] Use G1GC for tests

2017-10-03 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4748 For speed or consistency? ---

[GitHub] flink issue #4755: [hotfix] [docs] Fix broken links

2017-10-03 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4755 +1 for master and 1.3 ---

[GitHub] flink issue #4562: [FLINK-7402] Fix ineffective null check in NettyMessage#w...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4562 Merging ... ---

[GitHub] flink issue #4676: [FLINK-7625] Fix typo in metrics part of documents

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4676 Merging ... ---

[GitHub] flink issue #4575: [FLINK-7494][travis] Add license headers to '.travis.yml'...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4575 Merging ... ---

[GitHub] flink issue #4641: [hotfix][docs] Fix a typo on log name for quick start gui...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4641 Merging ... ---

[GitHub] flink pull request #4624: [FLINK-7410] [table] Use toString method to displa...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4624#discussion_r139187009 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/functions/UserDefinedFunction.scala --- @@ -41,7 +41,7 @@ abstract class

[GitHub] flink pull request #3511: [Flink-5734] code generation for normalizedkey sor...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/3511#discussion_r139186699 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java --- @@ -115,6 +115,13 @@ public Configuration

[GitHub] flink pull request #4574: [FLINK-6864] Fix confusing "invalid POJO type" mes...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4574#discussion_r139185112 --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java --- @@ -1900,7 +1900,8 @@ private boolean isValidPojoField

[GitHub] flink pull request #4574: [FLINK-6864] Fix confusing "invalid POJO type" mes...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4574#discussion_r139184023 --- Diff: docs/dev/types_serialization.md --- @@ -115,6 +115,8 @@ conditions are fulfilled: or have a public getter- and a setter- method

[GitHub] flink issue #4346: [FLINK-7199] [gelly] Graph simplification does not set pa...

2017-08-30 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4346 @NicoK I have fixed the noted issues and will merge. --- 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

[GitHub] flink issue #4574: [FLINK-6864] Fix confusing "invalid POJO type" messages f...

2017-08-30 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4574 I also agree that these messages have been personally useful. I think this PR is an improvement but that we could do better to note and/or emphasize the performance implications of `GenericType

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 @StephanEwen why are `null` values permitted if not in the contract of the input formats? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4529 @NicoK what do you think of creating a parent issue for your collection of network improvements? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink issue #4480: [FLINK-6995] [docs] Enable is_latest attribute to false

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4480 @tzulitai has this been merged? --- 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

[GitHub] flink issue #4535: Eventhubs-support read from and write to Azure eventhubs

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4535 @tzulitai this does sound like a good candidate for inclusion in Flink. Perhaps a separate `flink-connectors` repo would work better than Apache Bahir. --- If your project is set up for it, you

[GitHub] flink issue #4527: [FLINK-7409] [web] Make WebRuntimeMonitor reactive

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4527 @tillrohrmann please rebase now that #4492 has been merged --- 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

[GitHub] flink issue #4535: Eventhubs-support read from and write to Azure eventhubs

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4535 @zhuganghuaonnet we should also mention that new connectors are being contributed through the Flink release of [Apache Bahir](http://bahir.apache.org). --- If your project is set up for it, you

[GitHub] flink issue #4539: [FLINK-7443] [metrics] MetricFetcher store and deserializ...

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4539 +1 but I'm only seeing an improvement and not a bug --- 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

[GitHub] flink issue #4542: [FLINK-7445] [GitHub] Remove FLINK-1234 reference from PR...

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4542 +1 --- 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

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 `MutableObjectIterator#next` allows/requires a `null` result: "@return The object or null if the iterator is exhausted.". The documentation could be improved, e.g. in `MergeIterator` th

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 As I understand Flink does not allow `null` records since what does it mean to partition or window a null record? `null` fields are sometimes allowed but `null` records are meaningless

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-11 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 Hi @XuPingyong thanks for submitting this PR. I'm not clear on why the `null` check was added in FLINK-4075 and `ContinuousFileProcessingTest` is not running locally for me. When calling

[GitHub] flink pull request #4472: FLINK-7368: MetricStore makes cpu spin at 100%

2017-08-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4472#discussion_r132429138 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/metrics/MetricStore.java --- @@ -24,8 +24,8 @@ import org.slf4j.Logger

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-08-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r132427211 --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/parameter/LongParameter.java --- @@ -52,16 +52,6 @@ public

[GitHub] flink issue #4504: [FLINK-7395] [metrics] Count bytesIn/Out without synchron...

2017-08-09 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4504 @zentol what is the root cause for this change? I see that this is marked as a blocker bug. Was this reported or discussed on the mailing list? --- If your project is set up for it, you can reply

[GitHub] flink issue #4494: [FLINK-7026] Introduce flink-shaded-asm-5

2017-08-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4494 +1 LGTM --- 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

[GitHub] flink pull request #4461: [FLINK-7350] [travis] Only execute japicmp in misc...

2017-08-02 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4461#discussion_r130957157 --- Diff: tools/travis_mvn_watchdog.sh --- @@ -145,8 +150,11 @@ esac # -nsu option forbids downloading snapshot artifacts. The only snapshot

[GitHub] flink issue #4316: [FLINK-6105] Use InterruptedIOException instead of IOExce...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4316 @zhangminglei, not sure if you saw @tedyu's comment on the Jira. There are more instances of this in the same file and package. If these are internal classes and Flink is not using

[GitHub] flink issue #4461: [FLINK-7350] [travis] Only execute japicmp in misc profil...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4461 Should we upgrade to japicmp 0.10 or delay this change until such time? --- 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

[GitHub] flink issue #4460: [FLINK-7349] [travis] Only execute checkstyle in misc pro...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4460 +1; looks to be 20 to 30 minutes faster than recent test jobs on master --- 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

[GitHub] flink issue #4456: [FLINK-6996][kafka] Increase Xmx for tests

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4456 +1 this matches the parent `pom.xml`. Wondering if the same change would fix `flink-hbase` always failing for me when running `mvn verify`. --- If your project is set up for it, you can reply

[GitHub] flink issue #4346: [FLINK-7199] [gelly] Graph simplification does not set pa...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4346 @NicoK, parallelism tests have been added. There is a small refactor for `Runner` which needs more extensive rework elsewhere. --- If your project is set up for it, you can reply to this email

[GitHub] flink issue #4427: [FLINK-7304] [scripts] Simplify taskmanager GC configurat...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4427 +1 --- 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

[GitHub] flink issue #4426: [FLINK-7303] [build] Build elasticsearch5 by default

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4426 +1 --- 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

[GitHub] flink issue #4447: [FLINK-7312][checkstyle] activate checkstyle for flink/co...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4447 @StephanEwen we can search the diffs for commits with `checkstyle` in the summary for removal of `final`, which only yields ~100 results with only a handful requiring reversion. --- If your

[GitHub] flink pull request #4457: [hotfix] [gelly] Explicit type can be replaced wit...

2017-08-01 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4457 [hotfix] [gelly] Explicit type can be replaced with <> In Java 8 the diamond operator can be used in cases which would result in an error in Java 7. In Gelly we have often desired

[GitHub] flink issue #4447: [FLINK-7312][checkstyle] activate checkstyle for flink/co...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4447 @NicoK, just wanted to suggest that (other than trivial or single file modifications) to have separate commits for trailing whitespace and import order, then one or more checkstyle commits

[GitHub] flink issue #4400: [FLINK-7253] [tests] Remove CommonTestUtils#assumeJava8

2017-07-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4400 Changes look good but unused imports checkstyle violation. --- 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

[GitHub] flink issue #4399: [FLINK-7250] [build] Remove jdk8 profile

2017-07-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4399 LGTM --- 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

[GitHub] flink issue #4398: [FLINK-7249] [build] Bump java.version property to 1.8

2017-07-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4398 +1 to merge We can also simplify setting the garbage collector in `flink-dist/src/main/flink-bin/bin/taskmanager.sh` and including the `flink-connector-elasticsearch5` module in `flink

[GitHub] flink issue #4360: [FLINK-7220] [checkpoints] Update RocksDB dependency to 5...

2017-07-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4360 @uce @StephanEwen we had a bit of continued discussion in the JIRA. Can we add a pre-commit hook to the Apache repo to validate the format of the summary line? --- If your project is set up

[GitHub] flink issue #4360: [FLINK-7220] [checkpoints] Update RocksDB dependency to 5...

2017-07-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4360 The merge looks to have not been squashed. --- 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

[GitHub] flink issue #4398: [FLINK-7249] [build] Bump java.version property to 1.8

2017-07-27 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4398 Also need to update the documentation. $ grep -lir "java 7" docs/ docs//quickstart/java_api_quickstart.md docs//quickstart/scala_api_quickstart.md docs//

[GitHub] flink pull request #4405: [FLINK-7273] [gelly] Gelly tests with empty graphs

2017-07-26 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4405 [FLINK-7273] [gelly] Gelly tests with empty graphs ## What is the purpose of the change This was prompted by user feedback comparing Gelly's `PageRank` which noticed that zero-degree

[GitHub] flink issue #4393: [FLINK-7254] [java8] Properly activate checkstyle for fli...

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4393 +1 --- 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

[GitHub] flink issue #4394: [FLINK-7257] [runtime] Enable checkstyle for test files i...

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4394 +1 --- 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

[GitHub] flink issue #4388: [FLINK-7247] [travis] Replace java 7 build profiles

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4388 +1 test failures are due to network issues and look to have passed in your repo --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] flink pull request #4392: [FLINK-7226] [webUI] Properly include UTF-8 in con...

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4392#discussion_r129317438 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/handlers/JobCancellationWithSavepointHandlers.java --- @@ -266,7 +266,7

[GitHub] flink issue #4383: [hotfix] [optimizer] Normalize job plan operator formatti...

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4383 Dropping the space may make it easier to fix the line splitting issue in the web UI. @fhueske thoughts? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink issue #4396: [FLINK-7264] [travis] Kill watchdog only after compilatio...

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4396 +1 I think I had looked at this before and thought we were starting a second watchdog for the tests --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink pull request #4395: [FLINK-7263] Improve Pull Request template

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4395#discussion_r129307869 --- Diff: .github/PULL_REQUEST_TEMPLATE.md --- @@ -1,17 +1,71 @@ -Thanks for contributing to Apache Flink. Before you open your pull request, please

[GitHub] flink pull request #4395: [FLINK-7263] Improve Pull Request template

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4395#discussion_r129307702 --- Diff: .github/PULL_REQUEST_TEMPLATE.md --- @@ -1,17 +1,71 @@ -Thanks for contributing to Apache Flink. Before you open your pull request, please

[GitHub] flink pull request #4395: [FLINK-7263] Improve Pull Request template

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4395#discussion_r129309487 --- Diff: .github/PULL_REQUEST_TEMPLATE.md --- @@ -1,17 +1,71 @@ -Thanks for contributing to Apache Flink. Before you open your pull request, please

[GitHub] flink pull request #4395: [FLINK-7263] Improve Pull Request template

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4395#discussion_r129310148 --- Diff: .github/PULL_REQUEST_TEMPLATE.md --- @@ -1,17 +1,71 @@ -Thanks for contributing to Apache Flink. Before you open your pull request, please

[GitHub] flink issue #4374: repalce map.put with putIfAbsent

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4374 @RebornHuan please provide a brief description of the purpose of this change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] flink issue #4346: [FLINK-7199] [gelly] Graph simplification does not set pa...

2017-07-25 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4346 @NicoK thanks for the review! I'll work on refactoring the `Runner` and adding checks for checking the parallelism. I think we're okay if we whitelist the "large" operators by name. -

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r129296601 --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/InputBase.java --- @@ -32,6 +35,9 @@ extends

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r129286498 --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/parameter/Simplify.java --- @@ -109,21 +109,23 @@ public

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r129283124 --- Diff: flink-libraries/flink-gelly/src/main/java/org/apache/flink/graph/library/linkanalysis/HITS.java --- @@ -168,7 +168,6 @@ protected void

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-07-25 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r129282059 --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/input/InputBase.java --- @@ -32,6 +35,9 @@ extends

[GitHub] flink issue #4383: [hotfix] [optimizer] Normalize job plan operator formatti...

2017-07-24 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4383 @zentol I'd estimate that to be a similar change. --- 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

[GitHub] flink issue #4352: [FLINK-7211] [build] Exclude Gelly javadoc jar from relea...

2017-07-24 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4352 @aljoscha done. Thanks for the merge and managing the release! --- 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

[GitHub] flink pull request #4352: [FLINK-7211] [build] Exclude Gelly javadoc jar fro...

2017-07-24 Thread greghogan
Github user greghogan closed the pull request at: https://github.com/apache/flink/pull/4352 --- 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

[GitHub] flink issue #4388: [FLINK-7247] [travis] Replace java 7 build profiles

2017-07-24 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4388 Since we're running `trusty` it looks like we also have `openjdk8` available instead of using `oraclejdk8` for all tests. --- If your project is set up for it, you can reply to this email

[GitHub] flink issue #4385: wrong parameter check

2017-07-23 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4385 +1 --- 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

[GitHub] flink issue #4372: [FLINK-7234] [docs] Fix CombineHint documentation

2017-07-22 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4372 @fhueske I started running some benchmarks on HITS with each of the combiners (sort, hash, none) and at small scales hash is fasted followed by none with sort in last. None is somewhat faster than

[GitHub] flink pull request #4383: [hotfix] [optimizer] Normalize job plan operator f...

2017-07-21 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4383 [hotfix] [optimizer] Normalize job plan operator formatting When printing the job plan the operator description is typically formatted as the operator name followed by the user given or generated

[GitHub] flink issue #4372: [FLINK-7234] [docs] Fix CombineHint documentation

2017-07-21 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4372 @StephanEwen I like the new template. I much prefer free form over checkboxes. @fhueske I'm questioning my understanding of the the heuristic for using a hash-combine. For a fixed number

[GitHub] flink pull request #4372: [FLINK-7234] [docs] Fix CombineHint documentation

2017-07-19 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4372 [FLINK-7234] [docs] Fix CombineHint documentation ## What is the purpose of the change Update and correct the documentation for the use of CombineHint with `reduce` or `distinct

[GitHub] flink issue #4367: [FLINK-4499] [build] Add spotbugs plugin

2017-07-19 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4367 License isn't an issue since it's only used during the build: CAN APACHE PROJECTS DISTRIBUTE COMPONENTS UNDER PROHIBITED LICENSES? https://www.apache.org/legal/resolved.html#prohibited

[GitHub] flink issue #4350: [FLINK-7204] [core] CombineHint.NONE

2017-07-19 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4350 Test updated. --- 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

[GitHub] flink issue #4345: [FLINK-7197] [gelly] Missing call to GraphAlgorithmWrappi...

2017-07-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4345 @tedyu does this look right to you now? --- 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

[GitHub] flink pull request #4359: [FLINK-7140][blob] add an additional random compon...

2017-07-18 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4359#discussion_r128129730 --- Diff: flink-core/src/main/java/org/apache/flink/util/AbstractID.java --- @@ -46,7 +46,7 @@ /** The lower part of the actual ID

[GitHub] flink issue #4360: [FLINK-7220] [checkpoints] Update RocksDB dependency to 5...

2017-07-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4360 RocksDB 5.5.5 looks to include the desired licensing. --- 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

[GitHub] flink issue #4350: [FLINK-7204] [core] CombineHint.NONE

2017-07-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4350 @fhueske updated and manually verified. I had misinterpreted `DriverStrategy` to imply that `SORTED_REDUCE` would not use a combiner. --- If your project is set up for it, you can reply

[GitHub] flink pull request #4352: [FLINK-7211] [build] Exclude Gelly javadoc jar fro...

2017-07-17 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4352 [FLINK-7211] [build] Exclude Gelly javadoc jar from release You can merge this pull request into a Git repository by running: $ git pull https://github.com/greghogan/flink

[GitHub] flink pull request #4350: [FLINK-7204] [core] CombineHint.NONE

2017-07-16 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4350 [FLINK-7204] [core] CombineHint.NONE FLINK-3477 added a hash-combine preceding the reducer configured with CombineHint.HASH or CombineHint.SORT (default). In some cases it may be useful

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-07-14 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4346 [FLINK-7199] [gelly] Graph simplification does not set parallelism The Simplify parameter should accept and set the parallelism when calling the Simplify algorithms. You can merge this pull

[GitHub] flink pull request #4345: [FLINK-7197] [gelly] Missing call to GraphAlgorith...

2017-07-14 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4345#discussion_r127532604 --- Diff: flink-libraries/flink-gelly/src/main/java/org/apache/flink/graph/asm/translate/TranslateGraphIds.java --- @@ -56,7 +56,9 @@ public

[GitHub] flink pull request #4345: [FLINK-7197] [gelly] Missing call to GraphAlgorith...

2017-07-14 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4345 [FLINK-7197] [gelly] Missing call to GraphAlgorithmWrappingBase#canMergeConfigurationWith() Fix for methods calling the incorrect super function. You can merge this pull request into a Git

[GitHub] flink issue #3703: [FLINK-5005] WIP: publish scala 2.12 artifacts

2017-07-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/3703 @joan38 there has been a discussion on the mailing list about dropping Java 7 support (no one has objected) which will make it simpler to support Scala 2.12 in the upcoming release. --- If your

[GitHub] flink issue #4343: [FLINK-7190] Activate checkstyle flink-java/*

2017-07-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4343 What if we first split the suppressions files as with `suppressions-runtime.xml`? I think we could do this in one ticket for all of `core`, `java`, and `optimizer`. --- If your project is set up

[GitHub] flink issue #4323: [FLINK-7175] Add first simplest Flink benchmark

2017-07-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4323 Another idea: a separate repo but never released since this looks to be for observation by developers rather than the community at large (results could be summarized in the documentation

[GitHub] flink issue #4323: [FLINK-7175] Add first simplest Flink benchmark

2017-07-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4323 JMH's GPLv2 license is not compatible with the ASL (see 2063fa12)? This would need to be a separate repo distinct from the Apache Flink project and licensed under the GPL. --- If your project

[GitHub] flink issue #4305: [FLINK-7161] fix misusage of Float.MIN_VALUE and Double.M...

2017-07-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4305 +1 --- 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

[GitHub] flink pull request #4305: [FLINK-7161] fix misusage of Float.MIN_VALUE and D...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4305#discussion_r127109325 --- Diff: flink-core/src/main/java/org/apache/flink/configuration/Configuration.java --- @@ -843,11 +843,14 @@ private float convertToFloat(Object o

[GitHub] flink pull request #4313: [FLINK-7154] [docs] Missing call to build CsvTable...

2017-07-12 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/4313 [FLINK-7154] [docs] Missing call to build CsvTableSource example The Java and Scala example code for CsvTableSource create a builder but are missing the final call to build. You can merge

[GitHub] flink issue #4233: [FLINK-7047] [travis] Reorganize build profiles

2017-07-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4233 @aljoscha true. Now that proportionally more time is spent compiling we may be able to enable parallel builds for the compilation phase. I think most issues were with running tests where

[GitHub] flink issue #4009: [FLINK-6732] Activate strict-checkstyle for flink-java

2017-07-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4009 Okay, I now see there are many outstanding violations which look to be the type fixable by regex. If we split out PRs by package we should by able to copy over the comments updates from this PR

[GitHub] flink issue #4233: [FLINK-7047] [travis] Reorganize build profiles

2017-07-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4233 +1; total run time has dropped by almost half --- 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

[GitHub] flink issue #4009: [FLINK-6732] Activate strict-checkstyle for flink-java

2017-07-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4009 @zentol I believe this is the only outstanding checkstyle PR. Is this still in your review queue? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4295#discussion_r126949837 --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/KMeansWithBroadcastSetITCase.java --- @@ -65,7 +68,7 @@ public Centroid map(String c

[GitHub] flink issue #4293: [FLINK-7141][build] enable travis cache again

2017-07-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4293 Not only timeouts but we're likely downloading multiple gigabytes across the 12 builds per TravisCI job. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink pull request #4305: [FLINK-7161] fix misusage of Float.MIN_VALUE and D...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4305#discussion_r126946827 --- Diff: flink-core/src/main/java/org/apache/flink/configuration/Configuration.java --- @@ -843,11 +843,14 @@ private float convertToFloat(Object o

[GitHub] flink pull request #4110: [FLINK-6900] [metrics] Limit size of metric name c...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4110#discussion_r126930326 --- Diff: docs/monitoring/metrics.md --- @@ -376,6 +376,7 @@ Parameters: - `dmax` - hard limit for how long an old metric should be retained

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4295#discussion_r126928245 --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/RescalingITCase.java --- @@ -1035,4 +1040,43 @@ public void restoreState(Integer

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4295#discussion_r126928311 --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/StreamCheckpointNotifierITCase.java --- @@ -324,7 +322,7 @@ public void

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4295#discussion_r126928351 --- Diff: flink-tests/src/test/java/org/apache/flink/test/misc/SuccessAfterNetworkBuffersFailureITCase.java --- @@ -168,6 +173,6 @@ private static void

[GitHub] flink pull request #4295: [FLINK-6731] [tests] Activate strict checkstyle fo...

2017-07-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4295#discussion_r126928229 --- Diff: flink-tests/src/test/java/org/apache/flink/test/accumulators/AccumulatorIterativeITCase.java --- @@ -30,6 +29,9 @@ import

  1   2   3   4   5   6   7   8   9   10   >