[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[StephanEwen]

Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1082#issuecomment-138241493
  
Looks good. Merging this to "master" and "0.10-milestone1"


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/1082


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[rmetzger]

Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1082#issuecomment-13154
  
Thank you for the feedback. I will address the concerns.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[StephanEwen]

Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1082#issuecomment-136665284
  
Looks pretty good in general. Minor comments inline.

We have now the `Partitioner`, `KafkaPartitioner`, `RichKafkaPartitioner` 
classes. Can we somehow collapse the later two into one?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[rmetzger]

Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1082#issuecomment-136669637
  
I updated the PR and rebased to master (which is adding some commits from 
the future (at least from GitHubs perspective) )


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[StephanEwen]

Github user StephanEwen commented on a diff in the pull request:

https://github.com/apache/flink/pull/1082#discussion_r38404460
  
--- Diff: 
flink-staging/flink-streaming/flink-streaming-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/TestFixedPartitioner.java
 ---
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.streaming.connectors.kafka;
+
+import org.junit.Assert;
+import org.junit.Test;
+import 
org.apache.flink.streaming.connectors.kafka.partitioner.FixedPartitioner;
+
+public class TestFixedPartitioner {
+
+
+   /**
+*  Flink Sinks:Kafka Partitions
--- End diff --

Preformatting


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[StephanEwen]

Github user StephanEwen commented on a diff in the pull request:

https://github.com/apache/flink/pull/1082#discussion_r38404434
  
--- Diff: 
flink-staging/flink-streaming/flink-streaming-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/partitioner/RichKafkaPartitioner.java
 ---
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.streaming.connectors.kafka.partitioner;
+
+
+/**
+ * Extended Kafka Partitioner.
+ * It contains a prepare() method which is called on each parallel 
instance.
+ */
+public abstract class RichKafkaPartitioner implements KafkaPartitioner {
+   private static final long serialVersionUID = -4590784174150709918L;
+
+   public abstract void prepare(int parallelInstanceId, int 
parallelInstances, int[] partitions);
--- End diff --

In all other cases, the method is called `open()`. I think with that we 
established a bit of a terminology inside Flink, which would be good to follow 
here.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[StephanEwen]

Github user StephanEwen commented on a diff in the pull request:

https://github.com/apache/flink/pull/1082#discussion_r38404379
  
--- Diff: 
flink-staging/flink-streaming/flink-streaming-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/partitioner/FixedPartitioner.java
 ---
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.streaming.connectors.kafka.partitioner;
+
+/**
+ * A partitioner ensuring that each internal Flink partition ends up in 
one Kafka partition.
+ *
+ * Note, one Kafka partition can contain multiple Flink partitions.
+ *
+ * Cases:
+ * # More Flink partitions than kafka partitions
--- End diff --

These comments should be preformatted ``


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2372] Add new FlinkKafkaProducer

[rmetzger]

GitHub user rmetzger opened a pull request:

https://github.com/apache/flink/pull/1082

[FLINK-2372] Add new FlinkKafkaProducer

This pull request is reworking Flink's KafkaSink.

I've renamed the class to `FlinkKafkaProducer` to match it with 
`FlinkKafkaConsumer`.
It uses the new Kafka [Producer 
API](http://kafka.apache.org/documentation.html#producerapi).
In the documentation, they recommend using the new Producer API:

> As of the 0.8.2 release we encourage all new development to use the new 
Java producer. This client is production tested and generally both faster and 
more fully featured than the previous Scala client.

I've also noticed a good performance gain while doing some benchmarks with 
Flink and Kafka.

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

$ git pull https://github.com/rmetzger/flink flink2372-second

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

https://github.com/apache/flink/pull/1082.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 #1082


commit f042a6471f300e09894c521678a60aa144790134
Author: Robert Metzger 
Date:   2015-08-28T12:33:49Z

[FLINK-2372] Add new FlinkKafkaProducer




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---