KAFKA-6813: Remove deprecated APIs in KIP-182, Part II by guozhangwang · Pull Request #4976 · apache/kafka

guozhangwang · 2018-05-08T00:42:06Z

Remove the deprecated StateStoreSuppliers, and the corresponding Stores.create() functions and factories: only the base StateStoreSupplier and MockStoreSupplier were still preserved as they are needed by the deprecated TopologyBuilder and KStreamBuilder. Will remove them in a follow-up PR.
Add TopologyWrapper.java as the original InternalTopologyBuilderAccessor was removed, but I realized it is still needed as of now.
Minor: removed StateStoreTestUtils.java and inline its logic in its callers since now with StoreBuilder it is just a one-liner.

Committer Checklist (excluded from commit message)

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

…p120-Kip182-deprecated

…gy builder and kstream builder

…uozhangwang/kafka into K6813-part2-store-supplier

bbejeck

Thanks, @guozhangwang. Just a couple of minor comments otherwise LGTM.

bbejeck · 2018-05-08T20:50:23Z

    private final String name;
    private final long retentionPeriod;

+    private static final int NUM_SEGMENTS = 3;


Why the increase from 2 -> 3 ?

RocksDBSessionStoreSupplier.NUM_SEGMENTS is 3 -- it's not an increase here, is it?

bbejeck · 2018-05-08T20:51:45Z

    private final boolean retainDuplicates;

-    @SuppressWarnings("deprecation")
+    private static final int MIN_SEGMENTS = 2;


Should the MIN_SEGMENTS here be 3 as well or should the one above change?

RocksDBWindowStoreSupplier.MIN_SEMENTS is 2 -- note that MIN_SEGMENTS and NUM_SEGMENTS are two different things.

bbejeck · 2018-05-08T21:04:45Z

test failures in Java 8 unrelated.

retest this please.

mjsax · 2018-05-09T18:31:19Z

    private final String name;
    private final long retentionPeriod;

+    private static final int NUM_SEGMENTS = 3;


RocksDBSessionStoreSupplier.NUM_SEGMENTS is 3 -- it's not an increase here, is it?

mjsax · 2018-05-09T18:31:52Z

    private final boolean retainDuplicates;

-    @SuppressWarnings("deprecation")
+    private static final int MIN_SEGMENTS = 2;


RocksDBWindowStoreSupplier.MIN_SEMENTS is 2 -- note that MIN_SEGMENTS and NUM_SEGMENTS are two different things.

mjsax · 2018-05-09T18:39:22Z

        builder.addSource(null, "source", null, null, null, "topic");
        builder.addProcessor("processor", new MockProcessorSupplier(), "source");
-        builder.addStateStore(new MockStateStoreSupplier("store", true), "processor");
+        builder.addStateStore(storeBuilder, "processor");


the original store is persistent -- but storeBuilder is in-memory -- change intended?

For this test it does not matter. I was just reusing the store instance.

mjsax · 2018-05-09T18:44:16Z

        final long thirtySecondTimeout = 30 * 1000;

-        final TopologyBuilder builder = new TopologyBuilder()
+        final Topology topology = new TopologyWrapper()


use TopologyWrapper on left hand side and avoid cast below?

Unfortunately I cannot since once the addXX is called the returned type is Topology. But I refactored the code a bit to not use chained operators:

final TopologyWrapper topology = new TopologyWrapper(); topology.addSource("ingest", Pattern.compile("topic-\\d+")); topology.addProcessor("my-processor", processorSupplier, "ingest"); topology.addStateStore(storeBuilder, "my-processor");

…rt2-store-supplier

guozhangwang · 2018-05-09T18:55:43Z

@mjsax @bbejeck updated based on comments

mjsax · 2018-05-09T20:04:19Z

@guozhangwang Build failing with checkstyle error.

guozhangwang · 2018-05-09T20:22:46Z

@guozhangwang Build failing with checkstyle error.

ah right, thanks.

mjsax · 2018-05-09T23:45:43Z

Retest this please

mjsax

LGTM. Feel free to merge after Jenkins passed.

…-record-version * apache-github/trunk: KAFKA-6894: Improve err msg when connecting processor with global store (apache#5000) KAFKA-6893; Create processors before starting acceptor in SocketServer (apache#4999) MINOR: Fix typo in ConsumerRebalanceListener JavaDoc (apache#4996) MINOR: Remove deprecated valueTransformer.punctuate (apache#4993) MINOR: Update dynamic broker configuration doc for truststore update (apache#4954) KAFKA-6870 Concurrency conflicts in SampledStat (apache#4985) KAFKA-6361: Fix log divergence between leader and follower after fast leader fail over (apache#4882) KAFKA-6813: Remove deprecated APIs in KIP-182, Part II (apache#4976) KAFKA-6878 Switch the order of underlying.init and initInternal (apache#4988) KAFKA-6299; Fix AdminClient error handling when metadata changes (apache#4295) KAFKA-6878: NPE when querying global state store not in READY state (apache#4978) KAFKA 6673: Implemented missing override equals method (apache#4745) KAFKA-6834: Handle compaction with batches bigger than max.message.bytes (apache#4953)

1. Remove the deprecated StateStoreSuppliers, and the corresponding Stores.create() functions and factories: only the base StateStoreSupplier and MockStoreSupplier were still preserved as they are needed by the deprecated TopologyBuilder and KStreamBuilder. Will remove them in a follow-up PR. 2. Add TopologyWrapper.java as the original InternalTopologyBuilderAccessor was removed, but I realized it is still needed as of now. 3. Minor: removed StateStoreTestUtils.java and inline its logic in its callers since now with StoreBuilder it is just a one-liner. Reviewers: Bill Bejeck <bill@confluent.io>, Matthias J. Sax <matthias@confluent.io>

guozhangwang added 24 commits April 21, 2018 14:41

kstream and ktable deprecated stateful operators

56134c1

rebase from trunk

3d33190

remove deprecated apis in KGroupedTable and KGroupedStream

dfed53b

continue working on KStreamImpl and KTableImpl

0bd43c3

Merge branch 'trunk' of https://github.com/apache/kafka into K6813-Ki…

d7c90f6

…p120-Kip182-deprecated

continue working on KGroupedStreamImpl

c302aba

fix unit tests

81c05ca

fix two critical bugs

9592fc1

more fixes on unit tests

628069b

fixed all unit tests

a10e11c

a few minor cleanups

33fc44b

rebase from trunk

46df4a1

fix findbugs

0d5364c

Merge branch 'trunk' of https://github.com/apache/kafka into K6813-Ki…

eb9ab11

…p120-Kip182-deprecated

remove most of state store suppliers that are not dependent on topolo…

55728fd

…gy builder and kstream builder

github comments

52d3f1f

Merge branch 'K6813-Kip120-Kip182-deprecated' of https://github.com/g…

9567497

…uozhangwang/kafka into K6813-part2-store-supplier

rebase from trunk

3599d40

rebase from trunk

c27933e

minor fixes

98af3d6

revert to use mock state store supplier

192d3a2

fix unit tests

eb817f8

unused imports

225b81f

more unit test fixes

6e79d26

bbejeck approved these changes May 8, 2018

View reviewed changes

mjsax added the streams label May 9, 2018

mjsax reviewed May 9, 2018

View reviewed changes

guozhangwang added 2 commits May 9, 2018 11:52

Merge branch 'trunk' of https://github.com/apache/kafka into K6813-pa…

45f2526

…rt2-store-supplier

github comments

7b3f6fa

remove unused imports

03f1d91

mjsax approved these changes May 9, 2018

View reviewed changes

guozhangwang merged commit 0b1a118 into apache:trunk May 10, 2018

guozhangwang deleted the K6813-part2-store-supplier branch April 24, 2020 23:57

Conversation

guozhangwang commented May 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

bbejeck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbejeck commented May 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guozhangwang commented May 9, 2018

Uh oh!

mjsax commented May 9, 2018

Uh oh!

guozhangwang commented May 9, 2018

Uh oh!

mjsax commented May 9, 2018

Uh oh!

mjsax left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guozhangwang commented May 8, 2018 •

edited

Loading