You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/04/21 02:12:52 UTC
[GitHub] [kafka] ableegoldman opened a new pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
ableegoldman opened a new pull request #10573:
URL: https://github.com/apache/kafka/pull/10573
Deprecates and logs a warning upon usage of the following:
- StreamsConfig.EXACTLY_ONCE
- StreamsConfig.EXACTLY_ONCE_BETA
- Producer#sendOffsetsToTransaction(Map offsets, String consumerGroupId)
The deprecated eos configs are to be replaced by the new StreamsConfig.EXACTLY_ONCE_V2 config. Additionally, this PR replaces usages of the term "eos-beta" throughout the code with the term "eos-v2"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619556206
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
I feel like it does make more sense to include the actual variable here, since that's what's being deprecated. Also that way no one has to pause and think "hm, is that in StreamsConfig or ConsumerConfig or..."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r618912357
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -667,12 +668,14 @@ <h4><a class="toc-backref" href="#id30">probing.rebalance.interval.ms</a><a clas
<span id="streams-developer-guide-processing-guarantee"></span><h4><a class="toc-backref" href="#id25">processing.guarantee</a><a class="headerlink" href="#processing-guarantee" title="Permalink to this headline"></a></h4>
<blockquote>
<div>The processing guarantee that should be used.
- Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1),
- and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2).
- Using <code class="docutils literal"><span class="pre">"exactly_once"</span></code> requires broker
- version 0.11.0 or newer, while using <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code>
- requires broker version 2.5 or newer.
+ Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ and <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2).
+ Deprecated config options are <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS alpha),
Review comment:
I think given we have just rolled this out, and many users are likely still in the process of upgrading their brokers to a sufficient version, we should continue to mention EOS alpha. And then we may as well mention beta as well for consistency. Don't feel too strongly though
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619557404
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
+ from the release of 3.0, whichever comes last. Note that eos-v2 requires broker version 2.5 or higher, like eos-beta, so users should begin to upgrade their kafka cluster if necessary. See
Review comment:
From my perspective, it's ok to be flexible or to backtrack and push the removal date out further than we claimed, but not the other way around. In other words we may as well be aggressive and say we plan to remove it in 4.0 now, if it hasn't been a year by the time 4.0 rolls around or we aren't ready for whatever reason then we don't have to do it. Users will actually probably be happy to have more time since let's be real, many of them will not yet have upgraded their brokers 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r629845081
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
In general I agree, however given that we intend to remove it in 4.0 (that should not be too long out), it seems acceptable? If you feel strong about it, any proposal how to avoid it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619875764
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
Review comment:
I would be very bolt about it:
```
We deprecated <code>processing.guarantee</code> configuration value <code>"exactly_once"</code>
(for EOS version 1) in favor of the improved EOS version 2, formerly configured via
<code>"exactly_once_beta</code>. To avoid the confusion about the term "beta" in the config value
(it was never meant to imply it's not production ready), we furthermore renamed
<code>"exactly_once_beta"</code> to <code>"exactly_once_v2"</code>.
```
Or something similar.
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
Well, we do deprecate `StreamsConfig.EXACTLY_ONCE`, too, but user might just do `config.put("processing.guarantee", "exactly_once");` (or have a config file with `"exactly_once"`) in it. To me, the main change is that the config itself is deprecated and the deprecation of variable is just a "side effect".
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
+ from the release of 3.0, whichever comes last. Note that eos-v2 requires broker version 2.5 or higher, like eos-beta, so users should begin to upgrade their kafka cluster if necessary. See
Review comment:
Fair enough.
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -603,7 +606,7 @@ boolean runLoop() {
log.error("Shutting down because the Kafka cluster seems to be on a too old version. " +
"Setting {}=\"{}\" requires broker version 2.5 or higher.",
StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
- EXACTLY_ONCE_BETA);
+ StreamsConfig.EXACTLY_ONCE_V2);
Review comment:
SGTM -- if the required code changes to get the actual value are to much, I am fine with hard-coding the value, too.
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -111,6 +113,7 @@
private final StringSerializer stringSerializer = new StringSerializer();
private final ByteArraySerializer byteArraySerializer = new ByteArraySerializer();
+ private final UUID processId = UUID.randomUUID();
Review comment:
> only for eos-v2 for some reason
That is weird -- if `StreamsProducer` requires the `processID` it should have required it for `_beta` already? Would be good to understand -- maybe we unmasked a bug?
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -126,10 +129,10 @@ public void setup() {
clientSupplier.setCluster(cluster);
streamsProducer = new StreamsProducer(
config,
- "threadId",
+ processId + "-StreamThread-1",
Review comment:
This PR does not change `StreamsProducer` so this parsing should have happened for `_beta` already -- what do I miss?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r618911355
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -642,9 +647,13 @@ public void beginTransaction() throws ProducerFencedException {
* to the partition leader. See the exception for more details
* @throws KafkaException if the producer has encountered a previous fatal or abortable error, or for any
* other unexpected error
+ *
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #sendOffsetsToTransaction(Map, ConsumerGroupMetadata)} instead.
*/
+ @Deprecated
public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offsets,
String consumerGroupId) throws ProducerFencedException {
+ log.warn("This method has been deprecated and will be removed in 4.0, please use #sendOffsetsToTransaction(Map, ConsumerGroupMetadata) instead");
Review comment:
Oh I thought Ismael had said we were meant to do so during the KIP discussion, but I just re-read his message and I think he meant just for configs. I'll take this out
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r630601449
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
Done https://github.com/apache/kafka/pull/10675
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r618912905
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature,
+ you first need to upgrade your application to version 2.6.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
Review comment:
I just copied over the old instructions for upgrading to beta, and then mentioned how to upgrade to V2 down below. But since V2 is now the "real" version I suppose it should be the main one mentioned here
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature,
+ you first need to upgrade your application to version 2.6.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
+ and then do second round of rolling bounces to switch to <code>StreamsConfig.EXACTLY_ONCE_BETA</code>. If you
Review comment:
see comment above
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature,
+ you first need to upgrade your application to version 2.6.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
Review comment:
I just copied over the old instructions for upgrading to beta, and then mentioned how to upgrade to V2 down below. But since V2 is now the "real" version I suppose it should be the main one mentioned here. I'll update it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619876217
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
Well, we do deprecate `StreamsConfig.EXACTLY_ONCE`, too, but user might just do `config.put("processing.guarantee", "exactly_once");` (or have a config file with `"exactly_once"`) in it. To me, the main change is that the config itself is deprecated and the deprecation of variable is just a "side effect".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#issuecomment-825367328
@mjsax thanks for the review, addressed your comments please give it another pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#issuecomment-828751235
Tons of flaky test failures, all unrelated. Mostly Connect and Raft, a few Streams tests that are known to be flaky which I left some thoughts on the ticket for. Going to merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619558819
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ActiveTaskCreator.java
##########
@@ -115,7 +115,7 @@ public void reInitializeThreadProducer() {
StreamsProducer streamsProducerForTask(final TaskId taskId) {
if (processingMode != EXACTLY_ONCE_ALPHA) {
- throw new IllegalStateException("Producer per thread is used.");
+ throw new IllegalStateException("Expected EXACTLY_ONCE to be enabled, but the processing mode was " + processingMode);
Review comment:
I'll do it the other way around to be consistent with how we print the `processingMode` (ie change `eos-v2` to `EXACTLY_ONCE_V2` below instead)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r630552512
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
@ijuma I came across this after the question was raised around the autodowngrade logic, apparently (according to the config's javadocs) it's an "internal" config that's only used for Streams. The config itself is package-private.
Given that, I thought we may want to log a warning to any plain client users that saw this config and didn't notice that it was internal, and thus tried to use it. But I'm happy to do a followup PR to remove this. Alternatively, we can just take this config out -- I actually don't see any reason why it should be necessary, AFAICT it's just a slight convenience config that saves Streams from the ~5 lines of code it would take to do this downgrade itself (basically it just erases the extra consumer group metadata that isn't understood by older brokers). Not sure if this was vestigial from an older iteration of KIP-447, as it seems rather unnecessary..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619876369
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
+ from the release of 3.0, whichever comes last. Note that eos-v2 requires broker version 2.5 or higher, like eos-beta, so users should begin to upgrade their kafka cluster if necessary. See
Review comment:
Fair enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r617826977
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -642,9 +647,13 @@ public void beginTransaction() throws ProducerFencedException {
* to the partition leader. See the exception for more details
* @throws KafkaException if the producer has encountered a previous fatal or abortable error, or for any
* other unexpected error
+ *
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #sendOffsetsToTransaction(Map, ConsumerGroupMetadata)} instead.
*/
+ @Deprecated
public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offsets,
String consumerGroupId) throws ProducerFencedException {
+ log.warn("This method has been deprecated and will be removed in 4.0, please use #sendOffsetsToTransaction(Map, ConsumerGroupMetadata) instead");
Review comment:
Do we really need to log a WAR? We never did anything link this in KS code base in the past. Marking the method as `@Deprecated` should be sufficient IMHO? Or it such a WARN log custom in client code base?
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/Producer.java
##########
@@ -49,7 +49,10 @@
/**
* See {@link KafkaProducer#sendOffsetsToTransaction(Map, String)}
+ *
+
Review comment:
nit: needs cleanup
##########
File path: docs/streams/core-concepts.html
##########
@@ -291,16 +291,18 @@ <h2 class="anchor-heading"><a id="streams_processing_guarantee" class="anchor-li
commits on the input topic offsets, updates on the state stores, and writes to the output topics will be completed atomically instead of treating Kafka as an external system that may have side-effects.
For more information on how this is done inside Kafka Streams, see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-129%3A+Streams+Exactly-Once+Semantics">KIP-129</a>.<br />
- As of the 2.6.0 release, Kafka Streams supports an improved implementation of exactly-once processing, named "exactly-once beta",
+ As of the 2.6.0 release, Kafka Streams supports an improved implementation of exactly-once processing, named "exactly-once v2",
which requires broker version 2.5.0 or newer.
This implementation is more efficient, because it reduces client and broker resource utilization, like client threads and used network connections,
and it enables higher throughput and improved scalability.
+ As of the 3.0.0 release, the old "alpha" version of exactly-once has been deprecated. Users are encouraged to use exactly-once v2 for
Review comment:
We never used "alpha" anywhere in public, so might be better to avoid this term at all?
`old "alpha" -> "first"
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -667,12 +668,14 @@ <h4><a class="toc-backref" href="#id30">probing.rebalance.interval.ms</a><a clas
<span id="streams-developer-guide-processing-guarantee"></span><h4><a class="toc-backref" href="#id25">processing.guarantee</a><a class="headerlink" href="#processing-guarantee" title="Permalink to this headline"></a></h4>
<blockquote>
<div>The processing guarantee that should be used.
- Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1),
- and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2).
- Using <code class="docutils literal"><span class="pre">"exactly_once"</span></code> requires broker
- version 0.11.0 or newer, while using <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code>
- requires broker version 2.5 or newer.
+ Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ and <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2).
+ Deprecated config options are <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS alpha),
Review comment:
Do we need to elaborate on all deprecated configs?
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
Might be good to highlight at `beta -> v2` is just a renaming ?
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature,
+ you first need to upgrade your application to version 2.6.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
Review comment:
`you first need to upgrade your application to version 2.6.x`
Why would that be required? On can go to `3.0` directly from my understanding?
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -293,8 +293,9 @@ <h4><a class="toc-backref" href="#id5">bootstrap.servers</a><a class="headerlink
</tr>
<tr class="row-even"><td>processing.guarantee</td>
<td>Medium</td>
- <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1), or <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2)</td>.
+ <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ or <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2). Deprecated config options are
Review comment:
We should add `requires broker version 2.5 or newer`
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -293,8 +293,9 @@ <h4><a class="toc-backref" href="#id5">bootstrap.servers</a><a class="headerlink
</tr>
<tr class="row-even"><td>processing.guarantee</td>
<td>Medium</td>
- <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1), or <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2)</td>.
+ <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ or <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2). Deprecated config options are
+ <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS alpha) and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS beta)</td>.
Review comment:
don't use `alpha`; maybe just say `(requires broker version 0.11 or newer)
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -1010,18 +1026,51 @@ public StreamsConfig(final Map<?, ?> props) {
protected StreamsConfig(final Map<?, ?> props,
final boolean doLog) {
super(CONFIG, props, doLog);
- eosEnabled = StreamThread.eosEnabled(this);
+ eosEnabled = eosEnabled();
+
+ final String processingModeConfig = getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG);
+ if (processingModeConfig.equals(EXACTLY_ONCE)) {
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release. " +
+ "Please use `{}` instead. Note that this requires broker version 2.5+ so you should prepare "
+ + "to upgrade your brokers if necessary.", EXACTLY_ONCE, EXACTLY_ONCE_V2);
+ }
+ if (processingModeConfig.equals(EXACTLY_ONCE_BETA)) {
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release. " +
+ "Please use `{}` instead.", EXACTLY_ONCE_BETA, EXACTLY_ONCE_V2);
+ }
+
if (props.containsKey(RETRIES_CONFIG)) {
- log.warn("Configuration parameter `{}` is deprecated and will be removed in 3.0.0 release.", RETRIES_CONFIG);
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release.", RETRIES_CONFIG);
+ }
+ }
+
+ public ProcessingMode processingMode() {
+ if (EXACTLY_ONCE.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_ALPHA;
+ } else if (EXACTLY_ONCE_BETA.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_V2;
+ } else if (EXACTLY_ONCE_V2.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_V2;
+ } else {
+ return StreamThread.ProcessingMode.AT_LEAST_ONCE;
}
}
+ public boolean eosEnabled() {
Review comment:
This is a public API change -- to avoid this (ie also "leaks" and users won't need this method) we originally added it to `StreamThread` and not to `StreamsConfig` (even if it might be "cleaner" to have it on `StreamsConfig`)...
As an alternative, we would introduce `class InternalStreamsConfig extends StreamsConfig`, add the method their.... (If we do this, might be worth to split out a refactoring PR and not piggy-back on the KIP PR...)
Same for the other method below.
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature,
+ you first need to upgrade your application to version 2.6.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
+ and then do second round of rolling bounces to switch to <code>StreamsConfig.EXACTLY_ONCE_BETA</code>. If you
Review comment:
`EXACTLY_ONCE_BETA` or `EXACTLY_ONCE_V2`?
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 3.0.
Review comment:
`between 2.6 and 2.8` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r618914490
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -1010,18 +1026,51 @@ public StreamsConfig(final Map<?, ?> props) {
protected StreamsConfig(final Map<?, ?> props,
final boolean doLog) {
super(CONFIG, props, doLog);
- eosEnabled = StreamThread.eosEnabled(this);
+ eosEnabled = eosEnabled();
+
+ final String processingModeConfig = getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG);
+ if (processingModeConfig.equals(EXACTLY_ONCE)) {
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release. " +
+ "Please use `{}` instead. Note that this requires broker version 2.5+ so you should prepare "
+ + "to upgrade your brokers if necessary.", EXACTLY_ONCE, EXACTLY_ONCE_V2);
+ }
+ if (processingModeConfig.equals(EXACTLY_ONCE_BETA)) {
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release. " +
+ "Please use `{}` instead.", EXACTLY_ONCE_BETA, EXACTLY_ONCE_V2);
+ }
+
if (props.containsKey(RETRIES_CONFIG)) {
- log.warn("Configuration parameter `{}` is deprecated and will be removed in 3.0.0 release.", RETRIES_CONFIG);
+ log.warn("Configuration parameter `{}` is deprecated and will be removed in the 4.0.0 release.", RETRIES_CONFIG);
+ }
+ }
+
+ public ProcessingMode processingMode() {
+ if (EXACTLY_ONCE.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_ALPHA;
+ } else if (EXACTLY_ONCE_BETA.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_V2;
+ } else if (EXACTLY_ONCE_V2.equals(getString(StreamsConfig.PROCESSING_GUARANTEE_CONFIG))) {
+ return StreamThread.ProcessingMode.EXACTLY_ONCE_V2;
+ } else {
+ return StreamThread.ProcessingMode.AT_LEAST_ONCE;
}
}
+ public boolean eosEnabled() {
Review comment:
Ohh, I forgot this was public. Now the current code makes much more sense, I moved it because I thought it was so awkward. I'll just put it back and leave a comment so the next person doesn't fall into the same trap. Thanks for the explanation 😅
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619875764
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
Review comment:
I would be very bolt about it:
```
We deprecated <code>processing.guarantee</code> configuration value <code>"exactly_once"</code>
(for EOS version 1) in favor of the improved EOS version 2, formerly configured via
<code>"exactly_once_beta</code>. To avoid the confusion about the term "beta" in the config value
(it was never meant to imply it's not production ready), we furthermore renamed
<code>"exactly_once_beta"</code> to <code>"exactly_once_v2"</code>.
```
Or something similar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619558551
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -603,7 +606,7 @@ boolean runLoop() {
log.error("Shutting down because the Kafka cluster seems to be on a too old version. " +
"Setting {}=\"{}\" requires broker version 2.5 or higher.",
StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
- EXACTLY_ONCE_BETA);
+ StreamsConfig.EXACTLY_ONCE_V2);
Review comment:
My rationale was that if a user is hitting this then it's presumably a new application that they just tried to enable eos-v2 with, without upgrading their brokers. And if it's a new 3.0 application then why would they have chosen the deprecated eos-beta config over the eos-v2 config.
But I can try to print the actual result, it just requires a little extra code to look this up and save it since we don't track the configs after the thread's creation. Not the end of the world, but didn't feel necessary to me. Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r629848073
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
4.0 is probably 18 months away, that's a reasonably long time. Why do we need to log a warning only if it's not Streams?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619877817
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -111,6 +113,7 @@
private final StringSerializer stringSerializer = new StringSerializer();
private final ByteArraySerializer byteArraySerializer = new ByteArraySerializer();
+ private final UUID processId = UUID.randomUUID();
Review comment:
> only for eos-v2 for some reason
That is weird -- if `StreamsProducer` requires the `processID` it should have required it for `_beta` already? Would be good to understand -- maybe we unmasked a bug?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r629845576
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java
##########
@@ -179,10 +179,18 @@ public void beginTransaction() throws ProducerFencedException {
this.sentOffsets = false;
}
+ @SuppressWarnings("deprecation")
Review comment:
Would be better I guess. But doesn't it inherit the `@Deprecated` annotation automatically?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619554270
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 2.8.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature in version 3.0+,
+ you first need to upgrade your application to version 3.0.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
Review comment:
> If users have a config properties / text file, you would use the string.
I just thought it was clearer, but I didn't think about that. I'll revert this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619878083
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -126,10 +129,10 @@ public void setup() {
clientSupplier.setCluster(cluster);
streamsProducer = new StreamsProducer(
config,
- "threadId",
+ processId + "-StreamThread-1",
Review comment:
This PR does not change `StreamsProducer` so this parsing should have happened for `_beta` already -- what do I miss?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r628777957
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java
##########
@@ -179,10 +179,18 @@ public void beginTransaction() throws ProducerFencedException {
this.sentOffsets = false;
}
+ @SuppressWarnings("deprecation")
Review comment:
Shouldn't this have a deprecated annotation instead of the warning suppression?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r628777848
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
We should not have Streams specific code in the producer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r630553249
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java
##########
@@ -179,10 +179,18 @@ public void beginTransaction() throws ProducerFencedException {
this.sentOffsets = false;
}
+ @SuppressWarnings("deprecation")
Review comment:
I guess it doesn't need to have either, I can remove in a followup PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619559055
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -126,10 +129,10 @@ public void setup() {
clientSupplier.setCluster(cluster);
streamsProducer = new StreamsProducer(
config,
- "threadId",
+ processId + "-StreamThread-1",
Review comment:
StreamsProducer tries to parse the thread name to get the `processId` for the null check that's only done for eos-v2 (see above)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619877425
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -603,7 +606,7 @@ boolean runLoop() {
log.error("Shutting down because the Kafka cluster seems to be on a too old version. " +
"Setting {}=\"{}\" requires broker version 2.5 or higher.",
StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
- EXACTLY_ONCE_BETA);
+ StreamsConfig.EXACTLY_ONCE_V2);
Review comment:
SGTM -- if the required code changes to get the actual value are to much, I am fine with hard-coding the value, too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r620722055
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
Review comment:
Well, I kind of thought that we did intentionally choose to call it `beta` because we weren't completely confident in it when it was first released. But we are now, and looking back we can say with hindsight that it turned out to be production-ready back then. Still, I see your point, I'll make it more explicit with something like that suggestion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619558964
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -111,6 +113,7 @@
private final StringSerializer stringSerializer = new StringSerializer();
private final ByteArraySerializer byteArraySerializer = new ByteArraySerializer();
+ private final UUID processId = UUID.randomUUID();
Review comment:
StreamsProducer requires the processID to be non-null, but only for eos-v2 for some reason
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman merged pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman merged pull request #10573:
URL: https://github.com/apache/kafka/pull/10573
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619504008
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -642,7 +647,10 @@ public void beginTransaction() throws ProducerFencedException {
* to the partition leader. See the exception for more details
* @throws KafkaException if the producer has encountered a previous fatal or abortable error, or for any
* other unexpected error
+ *
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #sendOffsetsToTransaction(Map, ConsumerGroupMetadata)} instead.
Review comment:
I don't think we will be able to remove it in 4.0 and I would assume that 4.0 is too early, and we usually keep API around for at least 1 year after deprecation. Maybe best to just remove this part?
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -293,8 +293,9 @@ <h4><a class="toc-backref" href="#id5">bootstrap.servers</a><a class="headerlink
</tr>
<tr class="row-even"><td>processing.guarantee</td>
<td>Medium</td>
- <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1), or <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2)</td>.
+ <td colspan="2">The processing mode. Can be either <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ or <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2, requires broker version 2.5+). Deprecated config options are
+ <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1) and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS beta, requires broker version 2.5+)</td>.
Review comment:
nit `for EOS beta` -> `for EOS version 2`
(Might be really good to get rid of the term "beta" whenever we can -- and also make it more clear that we just renamed this config.)
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
+ from the release of 3.0, whichever comes last. Note that eos-v2 requires broker version 2.5 or higher, like eos-beta, so users should begin to upgrade their kafka cluster if necessary. See
Review comment:
Personally, I would not talk about when we plan to remove stuff, because the plan might change.
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -291,23 +293,35 @@
* <p>
* Enabling exactly-once processing semantics requires broker version 0.11.0 or higher.
* If you enable this feature Kafka Streams will use more resources (like broker connections)
- * compared to the {@link #AT_LEAST_ONCE} case.
+ * compared to {@link #AT_LEAST_ONCE "at_least_once"} and {@link #EXACTLY_ONCE_V2 "exactly_once_v2"}.
*
- * @see #EXACTLY_ONCE_BETA
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #EXACTLY_ONCE_V2 "exactly_once_v2"} instead.
*/
@SuppressWarnings("WeakerAccess")
+ @Deprecated
public static final String EXACTLY_ONCE = "exactly_once";
/**
* Config value for parameter {@link #PROCESSING_GUARANTEE_CONFIG "processing.guarantee"} for exactly-once processing guarantees.
* <p>
* Enabling exactly-once (beta) requires broker version 2.5 or higher.
- * If you enable this feature Kafka Streams will use less resources (like broker connections)
- * compare to the {@link #EXACTLY_ONCE} case.
+ * If you enable this feature Kafka Streams will use fewer resources (like broker connections)
+ * compared to the {@link #EXACTLY_ONCE} case.
Review comment:
`{@link #EXACTLY_ONCE} (deprecated) case.`
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -667,12 +668,14 @@ <h4><a class="toc-backref" href="#id30">probing.rebalance.interval.ms</a><a clas
<span id="streams-developer-guide-processing-guarantee"></span><h4><a class="toc-backref" href="#id25">processing.guarantee</a><a class="headerlink" href="#processing-guarantee" title="Permalink to this headline"></a></h4>
<blockquote>
<div>The processing guarantee that should be used.
- Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1),
- and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2).
- Using <code class="docutils literal"><span class="pre">"exactly_once"</span></code> requires broker
- version 0.11.0 or newer, while using <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code>
- requires broker version 2.5 or newer.
+ Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ and <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2).
+ Deprecated config options are <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS alpha),
Review comment:
Fair enough.
##########
File path: docs/streams/developer-guide/config-streams.html
##########
@@ -667,12 +668,14 @@ <h4><a class="toc-backref" href="#id30">probing.rebalance.interval.ms</a><a clas
<span id="streams-developer-guide-processing-guarantee"></span><h4><a class="toc-backref" href="#id25">processing.guarantee</a><a class="headerlink" href="#processing-guarantee" title="Permalink to this headline"></a></h4>
<blockquote>
<div>The processing guarantee that should be used.
- Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default),
- <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS version 1),
- and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS version 2).
- Using <code class="docutils literal"><span class="pre">"exactly_once"</span></code> requires broker
- version 0.11.0 or newer, while using <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code>
- requires broker version 2.5 or newer.
+ Possible values are <code class="docutils literal"><span class="pre">"at_least_once"</span></code> (default)
+ and <code class="docutils literal"><span class="pre">"exactly_once_v2"</span></code> (for EOS version 2).
+ Deprecated config options are <code class="docutils literal"><span class="pre">"exactly_once"</span></code> (for EOS alpha),
+ and <code class="docutils literal"><span class="pre">"exactly_once_beta"</span></code> (for EOS beta).
Review comment:
as above: `beta` -> `version 2`
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -657,8 +665,10 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
* This method should be used when you need to batch consumed and produced messages
* together, typically in a consume-transform-produce pattern. Thus, the specified
* {@code groupMetadata} should be extracted from the used {@link KafkaConsumer consumer} via
- * {@link KafkaConsumer#groupMetadata()} to leverage consumer group metadata for stronger fencing than
- * {@link #sendOffsetsToTransaction(Map, String)} which only sends with consumer group id.
+ * {@link KafkaConsumer#groupMetadata()} to leverage consumer group metadata. This will provide
+ * stronger fencing than just supplying the consumerGroupId and passing in {@code new ConsumerGroupMetadata(consumerGroupId)},
Review comment:
nit `{@code consumerGroupId}`
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 2.8.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature in version 3.0+,
+ you first need to upgrade your application to version 3.0.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
+ and then do second round of rolling bounces to switch to <code>StreamsConfig.EXACTLY_ONCE_V2</code>. If you
+ are upgrading an EOS application from an older (pre-2.6) version to a version between 2.6 and 2.8, follow these
+ same steps but with the config <code>StreamsConfig.EXACTLY_ONCE_BETA</code> instead. No special steps are required
+ to upgrade an application using <code>StreamsConfig.EXACTLY_ONCE_BETA</code> from version 2.6+ to 3.0 or higher: you can
+ just replace the config during the rolling upgrade.
Review comment:
`replace the config` -> `change the config from <code>StreamsConfig.EXACTLY_ONCE_BETA</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> during the rolling upgrade.`
(It might sound redundant, but in upgrade notes one cannot be too precise.)
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -53,17 +53,19 @@ <h1>Upgrade Guide and API Changes</h1>
</ul>
<p>
- Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2, which is configurable by setting
- <code>processing.guarantee</code> to <code>"exactly_once_beta"</code>.
- <b>NOTE:</b> The <code>"exactly_once_beta"</code> processing mode is ready for production (<i>i.e.</i>, it's not "beta" software).
+ Starting in Kafka Streams 2.6.x, a new processing mode is available, named EOS version 2. This can be configured
+ by setting <code>StreamsConfig.PROCESSING_GUARANTEE</code> to <code>StreamsConfig.EXACTLY_ONCE_V2</code> for
+ application versions 3.0+, or setting it to <code>StreamsConfig.EXACTLY_ONCE_BETA</code> for versions between 2.6 and 2.8.
To use this new feature, your brokers must be on version 2.5.x or newer.
- A switch from <code>"exactly_once"</code> to <code>"exactly_once_beta"</code> (or the other way around) is
- only possible if the application is on version 2.6.x.
- If you want to upgrade your application from an older version and enable this feature,
- you first need to upgrade your application to version 2.6.x, staying on <code>"exactly_once"</code>,
- and then do second round of rolling bounces to switch to <code>"exactly_once_beta"</code>.
- For a downgrade, do the reverse: first switch the config from <code>"exactly_once_beta"</code> to
- <code>"exactly_once"</code> to disable the feature in your 2.6.x application.
+ If you want to upgrade your EOS application from an older version and enable this feature in version 3.0+,
+ you first need to upgrade your application to version 3.0.x, staying on <code>StreamsConfig.EXACTLY_ONCE</code>,
Review comment:
Why do we switch from `"exaclty_once"` to `StreamsConfig.EXACTLY_ONCE`?
If users have a config properties / text file, you would use the string...
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
Review comment:
> to highlight its production-readiness
Sound like it becomes production ready in 3.0 release?
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -291,23 +293,35 @@
* <p>
* Enabling exactly-once processing semantics requires broker version 0.11.0 or higher.
* If you enable this feature Kafka Streams will use more resources (like broker connections)
- * compared to the {@link #AT_LEAST_ONCE} case.
+ * compared to {@link #AT_LEAST_ONCE "at_least_once"} and {@link #EXACTLY_ONCE_V2 "exactly_once_v2"}.
*
- * @see #EXACTLY_ONCE_BETA
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #EXACTLY_ONCE_V2 "exactly_once_v2"} instead.
Review comment:
remove `will be removed in 4.0`
(similar elsewhere)
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
As above: should we use `"exaclty_once"` instead of `StreamConfig.EXACTLY_ONCE` ?
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -603,7 +606,7 @@ boolean runLoop() {
log.error("Shutting down because the Kafka cluster seems to be on a too old version. " +
"Setting {}=\"{}\" requires broker version 2.5 or higher.",
StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
- EXACTLY_ONCE_BETA);
+ StreamsConfig.EXACTLY_ONCE_V2);
Review comment:
We don't know if the user set `_beta` or `_v2` -- should we try to get the value from `StreamsConfig` to provide an error message that matches whatever the user specified?
##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/ResetPartitionTimeIntegrationTest.java
##########
@@ -91,6 +91,7 @@ public static void closeCluster() {
private static final int DEFAULT_TIMEOUT = 100;
private static long lastRecordedTimestamp = -2L;
+ @SuppressWarnings("deprecation")
Review comment:
nit (below) replace `_beta` with `_v2` (even if it does not avoid the need to suppress warnings...
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -111,6 +113,7 @@
private final StringSerializer stringSerializer = new StringSerializer();
private final ByteArraySerializer byteArraySerializer = new ByteArraySerializer();
+ private final UUID processId = UUID.randomUUID();
Review comment:
Why do we need to introduce this one?
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -122,7 +122,9 @@
* <li>{@link ConsumerConfig#PARTITION_ASSIGNMENT_STRATEGY_CONFIG "partition.assignment.strategy"} (<code>StreamsPartitionAssignor</code>) - Streams client will always use its own partition assignor</li>
* </ul>
*
- * If {@link #PROCESSING_GUARANTEE_CONFIG "processing.guarantee"} is set to {@link #EXACTLY_ONCE "exactly_once"}, Kafka Streams does not allow users to overwrite the following properties (Streams setting shown in parentheses):
+ * If {@link #PROCESSING_GUARANTEE_CONFIG "processing.guarantee"} is set to {@link #EXACTLY_ONCE_V2 "exactly_once_v2"},
+ * {@link #EXACTLY_ONCE "exactly_once"}, or {@link #EXACTLY_ONCE_BETA "exactly_once_beta"}, Kafka Streams does not
Review comment:
`{@link #EXACTLY_ONCE "exactly_once"} (deprecated), or {@link #EXACTLY_ONCE_BETA "exactly_once_beta"} (deprecated)`
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ActiveTaskCreator.java
##########
@@ -115,7 +115,7 @@ public void reInitializeThreadProducer() {
StreamsProducer streamsProducerForTask(final TaskId taskId) {
if (processingMode != EXACTLY_ONCE_ALPHA) {
- throw new IllegalStateException("Producer per thread is used.");
+ throw new IllegalStateException("Expected EXACTLY_ONCE to be enabled, but the processing mode was " + processingMode);
Review comment:
nit: `EXACTLY_ONCE` -> `eos-v1` (to align to `eos-v2` below)
##########
File path: streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java
##########
@@ -126,10 +129,10 @@ public void setup() {
clientSupplier.setCluster(cluster);
streamsProducer = new StreamsProducer(
config,
- "threadId",
+ processId + "-StreamThread-1",
Review comment:
Just hard code `"-StreamThread-1"` -- why do we need to prefix it? We used a hard-coded value before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619555279
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
Review comment:
Hm, I thought this phrasing did imply it was already production-ready and that we just changed the name to highlight that fact. Do you have suggestions for how to word this better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#issuecomment-828752351
Merged to trunk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r620725616
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
Review comment:
I guess I'll just try to mention StreamsConfig in there somewhere...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619557786
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -291,23 +293,35 @@
* <p>
* Enabling exactly-once processing semantics requires broker version 0.11.0 or higher.
* If you enable this feature Kafka Streams will use more resources (like broker connections)
- * compared to the {@link #AT_LEAST_ONCE} case.
+ * compared to {@link #AT_LEAST_ONCE "at_least_once"} and {@link #EXACTLY_ONCE_V2 "exactly_once_v2"}.
*
- * @see #EXACTLY_ONCE_BETA
+ * @deprecated Since 3.0.0, will be removed in 4.0. Use {@link #EXACTLY_ONCE_V2 "exactly_once_v2"} instead.
Review comment:
See comment above -- I'm ok with removing this for the `sendOffsetsToTransaction` since all users have to do there is update one line of code, but for eos that may require a broker upgrade I'd rather leave this in to encourage them to do so
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r630556204
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##########
@@ -525,6 +525,11 @@ private TransactionManager configureTransactionState(ProducerConfig config,
final int transactionTimeoutMs = config.getInt(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG);
final long retryBackoffMs = config.getLong(ProducerConfig.RETRY_BACKOFF_MS_CONFIG);
final boolean autoDowngradeTxnCommit = config.getBoolean(ProducerConfig.AUTO_DOWNGRADE_TXN_COMMIT);
+ // Only log a warning if being used outside of Streams, which we know includes "StreamThread-" in the client id
+ if (autoDowngradeTxnCommit && !clientId.contains("StreamThread-")) {
Review comment:
@ableegoldman Your suggestion to remove the config altogether seems best to me. We don't have a grace period for internal configs, that's why they're internal. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r630593372
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java
##########
@@ -179,10 +179,18 @@ public void beginTransaction() throws ProducerFencedException {
this.sentOffsets = false;
}
+ @SuppressWarnings("deprecation")
Review comment:
Actually no, we do get a warning if we don't have either annotation. I'll change it to `@Deprecated` then
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r620726308
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -603,7 +606,7 @@ boolean runLoop() {
log.error("Shutting down because the Kafka cluster seems to be on a too old version. " +
"Setting {}=\"{}\" requires broker version 2.5 or higher.",
StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
- EXACTLY_ONCE_BETA);
+ StreamsConfig.EXACTLY_ONCE_V2);
Review comment:
I'll just put in both of them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10573: KAFKA-12574: KIP-732, Deprecate eos-alpha and replace eos-beta with eos-v2
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10573:
URL: https://github.com/apache/kafka/pull/10573#discussion_r619557012
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -93,6 +95,12 @@ <h1>Upgrade Guide and API Changes</h1>
</p>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
+ <p>
+ The <code>StreamsConfig.EXACTLY_ONCE</code> and <code>StreamsConfig.EXACTLY_ONCE_BETA</code> configs have been deprecated, and a new <code>StreamsConfig.EXACTLY_ONCE_V2</code> config has been
+ introduced. This is the same feature as eos-beta, but renamed to highlight its production-readiness. Users of exactly-once semantics should plan to migrate to the eos-v2 config and prepare for the removal of the deprecated configs in 4.0 or after at least a year
+ from the release of 3.0, whichever comes last. Note that eos-v2 requires broker version 2.5 or higher, like eos-beta, so users should begin to upgrade their kafka cluster if necessary. See
Review comment:
We specifically voted on this in the KIP. Personally I was a bit hesitant when someone suggested it, but I think the motivation there was to give users some slight sense of urgency so they don't put off upgrading their code or brokers, and then complain if/when we remove it because they haven't done so yet. Not sure if you saw the WW3 thread that John started when he synced over the commit that removed all the deprecated methods in 3.0...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org