Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188349853
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/RegistryAvroDeserializationSchema.java
---
@@ -0,0 +1,82
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188321914
--- Diff:
flink-formats/flink-avro-confluent-registry/src/main/java/org/apache/flink/formats/avro/registry/confluent
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188355756
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroSerializer.java
---
@@ -275,9 +304,19 @@ else
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188355390
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroSerializer.java
---
@@ -99,9 +108,29
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188348636
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215 @@
+/*
--- End
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188328437
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188337378
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188336725
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188350196
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/RegistryAvroDeserializationSchema.java
---
@@ -0,0 +1,82
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188353074
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroSerializer.java
---
@@ -79,15 +87,16
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188347289
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215 @@
+/*
--- End
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188328926
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188349785
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/RegistryAvroDeserializationSchema.java
---
@@ -0,0 +1,82
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188338897
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188319278
--- Diff:
flink-formats/flink-avro-confluent-registry/src/main/java/org/apache/flink/formats/avro/registry/confluent
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188319368
--- Diff:
flink-formats/flink-avro-confluent-registry/src/main/java/org/apache/flink/formats/avro/registry/confluent
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188349118
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188329273
--- Diff:
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/AvroDeserializationSchema.java
---
@@ -0,0 +1,215
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5982
My gut feeling is that we don't need `WriteMode.OVERWRITE` in cases where
one wants such an atomic file creation...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6016
I am not yet convinced that this fixes an actual problem, whereas it is not
clear whether it may cause regression (exception traces or performance).
To me this is a case to not do
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5995#discussion_r188316052
--- Diff: flink-formats/flink-avro-confluent-registry/pom.xml ---
@@ -0,0 +1,94 @@
+
+
+http://maven.apache.org/POM/4.0.0
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5982
Good point about the renaming on `close()` in case close is called for
cleanup, rather than success.
We could follow the same semantics as in
[CheckpointStateOutputStream](https
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6016
I am not sure we should be doing it like this.
The point of discouraging `Class.newInstance()` is not to copy replace this
by a different call. That will not magically make anything
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6002
Good fix, thanks.
+1 to merge
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5995
Thanks, the main code looks good!
Unfortunately, this seems to wither break the compatibility with prior
savepoints (when Avro types were implicitly handled through Kryo, now bridged
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5982
Thanks for preparing this.
I looked at the `TwoPhraseFSDatautputStream` - maybe we can make this
simpler. Do we need the distinction between phases? Is it not enough to behave
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5963
Looks good, thanks.
+1 to merge
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5897
Agreed, let's add it to master as well...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5970
Looks good, thanks.
+1 to merge
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5834
Committers usually have a lot of different responsibilities (releases,
testing, helping users on mailing lists, working on roadmap features, etc.).
All that takes a lot of time. Reviewing PRs
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6000
Congrats on having PR number 6000!
This overlaps with #6001, which is a mit more comprehensive (but need some
improvements).
Would you be up to coordinate to make one joint PR?
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/6000#discussion_r187798729
--- Diff: docs/dev/stream/operators/windows.md ---
@@ -730,9 +730,9 @@ input
/* ... */
-public class MyProcessWindowFunction
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5999
This is helpful, thanks.
Merging...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5966
I agree, we need different key/truststores for the internal/external
connectivity. This PR was meant as a step in that direction, separating at
least within the SSL Utils the internal
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5970#discussion_r187797400
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/sca/UdfAnalyzerTest.java ---
@@ -62,6 +62,10 @@
@SuppressWarnings("s
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5970
One minor style comment, otherwise this is good to go!
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5897
From my side, +1 to merge this to `release-1.4` and `release-1.5`.
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5897
Forwarding some comment from @fhueske from JIRA:
> I tried to reproduce this issue for 1.5 but it seems to work.
>
> Flink 1.5 should be out soon (release ca
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5982
How about adding the method `createAtomically` or so, with otherwise the
same signature as the `create(Path, WriteMode)` method?
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5735
@aljoscha Would be interested in your opinion here.
This is basically one of two ways to improve the handling of default values:
1. Add a default value supplier on the state
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5977#discussion_r187639708
--- Diff:
flink-connectors/flink-connector-kafka-0.11/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer011.java
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5977#discussion_r187640541
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -71,6 +71,17 @@
@PublicEvolving
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5973
Merged in `master` in 3afa5eb3c47158086ab29012a835f96682a85d34
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5982
I think this fix here might not work for S3, because a rename() with the S3
file systems will actually trigger a copy (or even a download and upload), so
it is not a cheap operation
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5970#discussion_r187610790
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/sca/UdfAnalyzerTest.java ---
@@ -109,7 +112,7 @@ public String map(MyPojo value) throws
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5962
The comment to make at least one attempt sounds good.
If you refactor this a little bit, you can actually test it. Either make it
a static method to which you pass the timer service
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5970#discussion_r187591783
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/sca/UdfAnalyzerTest.java ---
@@ -109,7 +112,7 @@ public String map(MyPojo value) throws
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5970#discussion_r187589977
--- Diff:
flink-core/src/test/java/org/apache/flink/api/common/typeinfo/TypeInformationTest.java
---
@@ -61,7 +61,7 @@ public void
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5970#discussion_r187592909
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/sca/UdfAnalyzerTest.java ---
@@ -268,8 +274,8 @@ public void
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5987
Sorry, but can we take back a step and first agree on what we actually want
the behavior to be before jumping into an implementation?
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5958
If it turns out that we need to do a bit more design work on the
deserialization schema, we could incrementally fix the issue that triggered
this PR by actually extending
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5958
There are a few bigger design aspects that we need to agree upon:
- The `DeserializationSchema` is a shared common denominator of
serialization schemata. That's why it is in `flink
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5973
Merging this...
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5980#discussion_r187191338
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SingleLogicalSlot.java
---
@@ -159,10 +159,51 @@ public
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5980#discussion_r187189887
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SingleLogicalSlot.java
---
@@ -159,10 +159,51 @@ public
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5978
Before merging this, we need to
- diff the dependency trees between the versions
- clear all licenses for the dependency changes. put changes into the
NOTICE files
- check
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5975
+1
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5973#discussion_r187025268
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java ---
@@ -81,16 +85,62 @@ public static void
setSSLVerAndCipherSuites
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5972
+1 to the cleanup, but I think this might be breaking relative paths of the
suppression files, hence causing the build to fail. Probably need to make the
paths relative to "root dir&quo
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5936
Looks good, thanks.
+1 to merge this
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5966#discussion_r186800971
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/NettyServer.java
---
@@ -170,8 +171,8 @@ public void initChannel
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5966
@EronWright This might be interesting to you as well
---
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/5966
[FLINK-9312] [security] Add mutual authentication for RPC and data plane
## What is the purpose of the change
Currently, the Flink processes encrypted connections via SSL:
- Data
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5965
@EronWright This might be interesting to you.
---
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/5965
[FLINK-9310] [security] Update standard cipher suites for secure mode
## What is the purpose of the change
This sets the cipher suits accepted by default to those recommended
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5936
Yes, before giving +1 to this commit, we need to check that this introduces
no new transitive dependency, or need to make sure that dependency is not an
issue.
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5427
Thanks a lot, looks good, can merge this now.
One quick question: You decided to have empty default implementations for
the new methods in the master hook interface. Given that Pravega
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5954#discussion_r186376326
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/scheduler/SchedulerTestBase.java
---
@@ -296,7 +296,7 @@ public
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5954#discussion_r186376122
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPool.java
---
@@ -1070,13 +1071,22 @@ protected void
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5954#discussion_r186376182
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolGateway.java
---
@@ -86,9 +86,10 @@
* Releases
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5954#discussion_r186375693
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPool.java
---
@@ -212,7 +212,7 @@ public void start(JobMasterId
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5954#discussion_r186375841
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPool.java
---
@@ -1050,11 +1050,12 @@ else
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5931
@sihuazhou and @shuai-xu thank you for your help in understanding the bug
here.
Let me rephrase it to make sure I understand the problem exactly. The steps
are the following
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5951
Good catch! Thank you for the PR.
Will try to review this asap...
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5928#discussion_r185737131
--- Diff: docs/ops/state/state_backends.md ---
@@ -152,7 +152,7 @@ Possible values for the config entry are *jobmanager*
(MemoryStateBackend), *fil
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5928
In Flink 1.5, all checkpoints are externalized. There is no notion of
externalized checkpoints any more, just a setting to configure the retention
(retain on cancel, retain on fail, never retain
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5929
I am not sure if failing is the right behavior here.
What is someone has a Flink job running and one of the topics for which
partitions are discovered gets deleted? That would fail
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5936
Could you add the dependency tree of the upgraded Prometheus dependency to
check?
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5939
The feature is a nice addition.
Flink currently already adds the timestamp as the record's event time
timestamp. You can access it via a ProcessFunction. That is a tad bit more
clumsy
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5944#discussion_r185732563
--- Diff:
flink-yarn/src/main/java/org/apache/flink/yarn/entrypoint/YarnJobClusterEntrypoint.java
---
@@ -131,6 +133,17 @@ protected JobGraph
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5944#discussion_r185732507
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/MiniDispatcher.java
---
@@ -109,7 +119,11 @@ public MiniDispatcher
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5944#discussion_r185732539
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/dispatcher/MiniDispatcher.java
---
@@ -109,7 +119,11 @@ public MiniDispatcher
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5948#discussion_r185731303
--- Diff: docs/monitoring/debugging_classloading.md ---
@@ -69,9 +69,9 @@ cases. By default, Java ClassLoaders will first look for
classes in the parent
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5949
Looks good, thanks, merging this...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5900
Nice addition!
A few things I would like to double check on the quickstart configuration
(I am not fluent enough Gradle):
- We do not need to hide/shade any dependencies
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5914
Change looks very good, thanks!
Merging this...
We can probably remove most of this code out again later, once we drop the
non-credit-based code paths in the next releases
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5916
Merging...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5942
Good fix, thanks!
Merging...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5943
Good fix, thanks, merging...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5938
Merging this...
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5938#discussion_r185080370
--- Diff:
flink-clients/src/main/java/org/apache/flink/client/program/rest/RestClusterClient.java
---
@@ -570,6 +571,21 @@ public LeaderConnectionInfo
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5944
The test failure is unrelated - unrelated test flakeyness
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5931
@GJL Briefly digging through the log, there are a few strange things
happening:
- `YarnResourceManager` still has 8 pending requests even when 11
containers are running
GitHub user StephanEwen opened a pull request:
https://github.com/apache/flink/pull/5944
[FLINK-8900] [yarn] Set correct application status when job is finished
## What is the purpose of the change
When finite Flink applications (batch jobs) are sent to YARN
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5897
Can we actually get rid of `commons-configuration` in the table API?
All the commons packages with their weird long tail of not properly
declared dependencies have become a bit
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5924
Thanks, merging this...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5934
Concerning serializer snapshots:
- We need to move away from Java Serializing the serializers into the
config snapshots anyways and should do that in the near future.
- I think
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5892
Thanks, looks good, merging this...
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5928
The configuration (`config.md`)should be generated from the config options
by now, so not be manually edited.
(@zentol could you chime in here?)
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5928#discussion_r185021293
--- Diff: docs/dev/stream/state/checkpointing.md ---
@@ -137,11 +137,9 @@ Some more parameters and/or defaults may be set via
`conf/flink-conf.yaml
101 - 200 of 4019 matches
Mail list logo
