KAFKA-15307: Update/errors for deprecated config #14448

Cerchie · 2023-09-25T19:52:10Z

This PR is a follow-up on #14360.
It ensures that warnings are shown if the dep'd configs outlined in that PR are used (note topology.optimization's variable has changed rather than the configuration itself; this is already addressed in the code via KIP-626).

Tests have been added to StreamsConfigTest.java to ensure the proper warnings show up.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

Cerchie · 2023-09-25T19:52:50Z

tagging @mjsax for review

github-actions · 2023-12-25T03:33:11Z

This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch)

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

mjsax · 2024-01-17T23:48:41Z

Thanks for the PR. Overall LGTM.

I am just wondering if we should also update the JavaDocs for all deprecated configs? For example:

    /** {@code default.windowed.key.serde.inner} */
    @SuppressWarnings("WeakerAccess")
    @Deprecated
    public static final String DEFAULT_WINDOWED_KEY_SERDE_INNER_CLASS = "default.windowed.key.serde.inner";

We could change it to:

    /**
     * {@code default.windowed.key.serde.inner}
     * @deprecation Since X.y.0. Use {@link #WINDOWED_INNER_CLASS_SERDE} instead.
     */
    @SuppressWarnings("WeakerAccess")
    @Deprecated
    public static final String DEFAULT_WINDOWED_KEY_SERDE_INNER_CLASS = "default.windowed.key.serde.inner";

Thought? We could do this for all deprecated configs?

Cerchie · 2024-02-22T15:38:33Z

Thanks for the PR. Overall LGTM.

I am just wondering if we should also update the JavaDocs for all deprecated configs? For example:

    /** {@code default.windowed.key.serde.inner} */
    @SuppressWarnings("WeakerAccess")
    @Deprecated
    public static final String DEFAULT_WINDOWED_KEY_SERDE_INNER_CLASS = "default.windowed.key.serde.inner";

We could change it to:

    /**
     * {@code default.windowed.key.serde.inner}
     * @deprecation Since X.y.0. Use {@link #WINDOWED_INNER_CLASS_SERDE} instead.
     */
    @SuppressWarnings("WeakerAccess")
    @Deprecated
    public static final String DEFAULT_WINDOWED_KEY_SERDE_INNER_CLASS = "default.windowed.key.serde.inner";

Thought? We could do this for all deprecated configs?

went through and addressed all the missing ones I found

rebase-- adds NO_OPTIMIZATION codeblock as well as default.dsl.store paragraph

accept classutils change

Co-authored-by: Matthias J. Sax <mjsax@apache.org>

.github/workflows/codesee-arch-diagram.yml

docs/streams/developer-guide/config-streams.html

Cerchie · 2024-04-30T20:52:40Z

tagging @mjsax in for re-review

mjsax · 2024-05-16T06:14:18Z

@Cerchie -- this PR would need a rebase as #14360 was merged. Also, seems we introduced a few JavaDoc issues (cf #14360 (comment)) that we should fix in this PR.

mjsax · 2024-05-18T20:30:07Z

docs/streams/developer-guide/config-streams.html

            <td>10485760</td>
          </tr>
-          <tr class="row-even"><td>cache.max.bytes.buffering (Deprecated. Use statestore.cache.max.bytes instead.)</td>
+          <tr class="row-even"><td>cache.max.bytes.buffering (Deprecated. Use cache.max.bytes instead.)</td>


Seem statestore.cache.max.bytes is correct?

mjsax · 2024-05-18T20:31:15Z

docs/streams/developer-guide/config-streams.html

-          </tr>
-          <tr class="row-odd"><td>default.windowed.value.serde.inner (Deprecated. Use windowed.inner.class.serde instead.)</td>
+          <tr class="row-even"><td>default.windowed.key.serde.inner (Deprecated.)</td>
+


nit: remove empty line

mjsax · 2024-05-18T20:31:36Z

docs/streams/developer-guide/config-streams.html

-            <td><code class="docutils literal"><span class="pre">null</span></code></td>
-          </tr>
-          <tr class="row-odd"><td>default.windowed.value.serde.inner (Deprecated. Use windowed.inner.class.serde instead.)</td>
+          <tr class="row-even"><td>default.windowed.key.serde.inner (Deprecated.)</td>


Seems we don't need this line? It's duplicated above and below?

mjsax · 2024-05-18T20:33:07Z

docs/streams/developer-guide/config-streams.html

-          <tr class="row-odd"><td>default.windowed.value.serde.inner (Deprecated. Use windowed.inner.class.serde instead.)</td>
+          <tr class="row-even"><td>default.windowed.key.serde.inner (Deprecated.)</td>
+
+          <tr class="row-odd"><td>default.windowed.key.serde.inner (Deprecated. Use windowed.inner.class.serde instead.)</td>


duplicated line?

mjsax · 2024-05-18T20:33:54Z

docs/streams/developer-guide/config-streams.html

@@ -307,25 +307,23 @@ <h4><a class="toc-backref" href="#id23">num.standby.replicas</a><a class="header
              set by the user or all serdes must be passed in explicitly (see also default.key.serde).</td>
            <td><code class="docutils literal"><span class="pre">null</span></code></td>
          </tr>
-          <tr class="row-even"><td>default.windowed.key.serde.inner (Deprecated. Use windowed.inner.class.serde instead.)</td>


Why does this PR remove this row in the table?

mjsax · 2024-05-18T20:35:24Z

streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java

@@ -500,7 +500,9 @@ public class StreamsConfig extends AbstractConfig {
    private static final String BUILT_IN_METRICS_VERSION_DOC = "Version of the built-in metrics to use.";

    /** {@code cache.max.bytes.buffering}
+


nit: about unnecessary empty lines (same right below)

mjsax · 2024-05-18T20:36:52Z

streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java

@@ -658,7 +660,7 @@ public class StreamsConfig extends AbstractConfig {
    public static final String METRICS_SAMPLE_WINDOW_MS_CONFIG = CommonClientConfigs.METRICS_SAMPLE_WINDOW_MS_CONFIG;

    /** {@code auto.include.jmx.reporter}
-     * @deprecated and will be removed in 4.0.0 */
+     * @deprecated and will removed in 4.0.0 Users should instead include org.apache.kafka.common.metrics.JmxReporter in metric.reporters in order to enable the JmxReporter.} */


Suggested change

* @deprecated and will removed in 4.0.0 Users should instead include org.apache.kafka.common.metrics.JmxReporter in metric.reporters in order to enable the JmxReporter.} */

* @deprecated and be will removed in 4.0.0; include {@link org.apache.kafka.common.metrics.JmxReporter} via config {@code metric.reporters} in order to enable the JmxReporter. */

mjsax · 2024-05-18T20:44:17Z

docs/streams/developer-guide/config-streams.html

@@ -261,10 +261,10 @@ <h4><a class="toc-backref" href="#id23">num.standby.replicas</a><a class="header
          </tr>
          <tr class="row-odd"><td>statestore.cache.max.bytes</td>
            <td>Medium</td>
-            <td colspan="2">Maximum number of memory bytes to be used for record caches across all threads.</td>
+            <td colspan="2">Maximum number of memory bytes to be used for record caches across all threads. Note that at the debug level you can use <code>cache.size</code> to monitor the actual size of the Kafka Streams cache.</td>


Not clear to me, what the added sentence means? Are you referring to JMX metrics?

Cerchie mentioned this pull request Sep 25, 2023

KAFKA-15307: update/note deprecated configs #14360

Merged

3 tasks

yashmayya added the streams label Sep 25, 2023

github-actions bot added the stale Stale PRs label Dec 25, 2023

mjsax removed the stale Stale PRs label Jan 17, 2024

codesee-maps[bot] and others added 13 commits February 23, 2024 14:30

Install the CodeSee workflow. Learn more at https://docs.codesee.io

be7a8e7

note deprecated configs

116d232

rebase-- adds NO_OPTIMIZATION codeblock as well as default.dsl.store paragraph

remove space

0f27b90

move note re deprecation to headline

052e84b

accept classutils change

topology.optimization not deprecated, just variable, see KIP 626

2a5972f

add pointer to windowed.inner.class.serde

b31db17

adds windowed.inner.class.serde

8a37b49

add cache.max.bytes

6bc7440

ensure all dep'd configs have jdocs

2dc27c0

address comments and tweaks

1c87f05

Update streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java

3d5072d

Co-authored-by: Matthias J. Sax <mjsax@apache.org>

Update streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java

8b03671

Co-authored-by: Matthias J. Sax <mjsax@apache.org>

suggested links addeD

c353eda

Cerchie force-pushed the update/errors-for-deprecated-config branch from e2fab78 to c353eda Compare February 23, 2024 21:50

mjsax reviewed Mar 1, 2024

View reviewed changes

Cerchie added 4 commits April 25, 2024 13:03

remove codesee yml file

667572e

remove windowedinnerclassserde

2e67c8c

remove unresolved conflict markers

9592ae0

fix evens/odds

fb4a230

Cerchie requested a review from mjsax April 30, 2024 20:52

Cerchie and others added 2 commits May 16, 2024 13:28

close brackets and remove link to jmxreporter

79979d0

Merge branch 'trunk' into update/errors-for-deprecated-config

7747d7e

mjsax reviewed May 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-15307: Update/errors for deprecated config #14448

KAFKA-15307: Update/errors for deprecated config #14448

Cerchie commented Sep 25, 2023

Cerchie commented Sep 25, 2023

github-actions bot commented Dec 25, 2023

mjsax commented Jan 17, 2024

Cerchie commented Feb 22, 2024

Cerchie commented Apr 30, 2024

mjsax commented May 16, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

mjsax May 18, 2024

		@@ -500,7 +500,9 @@ public class StreamsConfig extends AbstractConfig {
		private static final String BUILT_IN_METRICS_VERSION_DOC = "Version of the built-in metrics to use.";

		/** {@code cache.max.bytes.buffering}

	* @deprecated and will removed in 4.0.0 Users should instead include org.apache.kafka.common.metrics.JmxReporter in metric.reporters in order to enable the JmxReporter.} */
	* @deprecated and be will removed in 4.0.0; include {@link org.apache.kafka.common.metrics.JmxReporter} via config {@code metric.reporters} in order to enable the JmxReporter. */

KAFKA-15307: Update/errors for deprecated config #14448

Are you sure you want to change the base?

KAFKA-15307: Update/errors for deprecated config #14448

Conversation

Cerchie commented Sep 25, 2023

Committer Checklist (excluded from commit message)

Cerchie commented Sep 25, 2023

github-actions bot commented Dec 25, 2023

mjsax commented Jan 17, 2024

Cerchie commented Feb 22, 2024

Cerchie commented Apr 30, 2024

mjsax commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment