feat(remote_wal): make meta srv able to alloc kafka topics #2753

niebayes · 2023-11-15T10:23:47Z

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

This PR is part of the Kafka remote wal project.
It adds Kafka stuff to meta srv which is now able to allocate Kafka topics for regions while handling create table requests.

Details

There resides a WalMetaAllocator in MetaSrvTablemetadataAllocator. When the create inferface of the MetaSrvTablemetadataAllocator is invoked, a number of topics will be allocated by WalMetaAllocator. Those topics will be assigned to regions when executing CreateTableProcedure by the procedure manager.
A region's topic is stored in RegionOptions so that it could be used in opening and creating regions by datanode.

Checklist

I have written the necessary rustdoc comments.
I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

codecov · 2023-11-16T02:38:07Z

Codecov Report

Merging #2753 (6087d92) into develop (102e43a) will decrease coverage by 0.43%.
Report is 1 commits behind head on develop.
The diff coverage is 50.79%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2753      +/-   ##
===========================================
- Coverage    84.68%   84.25%   -0.43%     
===========================================
  Files          730      735       +5     
  Lines       113711   113960     +249     
===========================================
- Hits         96291    96012     -279     
- Misses       17420    17948     +528

Cargo.toml

src/common/meta/src/ddl.rs

src/common/meta/src/ddl/create_table.rs

src/common/meta/src/wal/kafka/topic_manager.rs

src/common/meta/src/wal/kafka/topic_selector.rs

MichaelScofield

Tests are too few, especially the cases for kafka. I think it's a must to do it first.

Cargo.toml

src/common/meta/src/ddl/create_table.rs

src/common/meta/src/wal/kafka.rs

src/common/meta/src/wal/kafka/topic_manager.rs

MichaelScofield · 2023-11-16T03:50:12Z

src/common/meta/src/wal/kafka/topic_manager.rs

+                return None;
+            }
+
+            Some(kafka_ctrl_client.create_topic(


Does rskafka return an error if the topic already exist?

There's a TopicAlreadyExists Error in rskafka, but it's never used except in one test.
See: https://github.com/influxdata/rskafka/blob/a9a16d51eba40dbce7a5d976f2a7094dfc0a7e17/src/protocol/error.rs#L48

We need to further determine how rskafka handles an already-exist topic.

src/common/meta/src/wal/kafka/topic_selector.rs

niebayes · 2023-11-24T15:17:22Z

Waiting for #2816 to be merged.
There're alternative ways to store the kafka topics. This PR shall be merged when the scheme is finally determined.

WenyXu

For metadata, we need to consider the ability of backward compatibility; it's not easy to make breaking changes in the production environment.
I think Kafka WAL will be replaced with our own WAL service in the future. The code should be more general, and we can migrate to the new WAL service easily.

src/common/meta/src/ddl/create_table.rs

WenyXu · 2023-11-24T14:54:25Z

src/common/meta/src/error.rs

+    #[snafu(display("Failed to deserialize Kafka topics"))]
+    DeserKafkaTopics {
+        location: Location,
+        #[snafu(source)]
+        error: JsonError,
+    },
+
+    #[snafu(display("Failed to serialize Kafka topics"))]
+    SerKafkaTopics {
+        location: Location,
+        #[snafu(source)]
+        error: JsonError,
+    },


How about using DecodeJson and EncodeJson?

src/common/meta/src/wal/kafka/topic_manager.rs

src/common/meta/src/ddl/create_table.rs

src/common/meta/src/wal/kafka/topic_manager.rs

src/common/meta/src/wal/meta.rs

src/meta-srv/src/table_meta_alloc.rs

src/meta-srv/src/metasrv/builder.rs

config/metasrv.example.toml

src/common/meta/src/ddl.rs

src/common/meta/src/ddl/create_table.rs

src/common/meta/src/wal.rs

src/common/meta/src/wal/kafka/topic_manager.rs

killme2008 · 2023-11-28T10:02:32Z

src/common/meta/src/wal/kafka/topic_manager.rs

+        .await
+        .context(CreateKafkaTopicSnafu)?;
+
+    persist_created_topics(&topics, kv_backend).await?;


I think we should add some info logs for creating topics.

Will do so.

src/common/meta/src/wal/meta.rs

killme2008 · 2023-11-30T03:24:57Z

@niebayes There are some comments, are u currently working on addressing them? Let's discuss if you have any problems.

niebayes requested review from fengjiachun, MichaelScofield, v0y4g3r and evenyag and removed request for fengjiachun November 15, 2023 10:24

v0y4g3r reviewed Nov 16, 2023

View reviewed changes

MichaelScofield reviewed Nov 16, 2023

View reviewed changes

niebayes self-assigned this Nov 20, 2023

niebayes added 3 commits November 23, 2023 15:08

feat(remote_wal): make meta srv able to allocate kafka topics

988b2ba

fix(remote_wal): resolve review conversations

c328c43

feat(remote_wal): introduce wal meta to organize wal codes

e77f0fa

niebayes force-pushed the feat/remote_wal_meta_srv branch from a8a0f7e to e77f0fa Compare November 23, 2023 07:35

niebayes added 4 commits November 23, 2023 15:49

chore: add comments on wal options

35c55c5

fix: sqlness test

a878fe1

fix: create region bug in standalone mode

8ce066f

fix: properly handle topic allocation in standalone mode

6f41530

niebayes mentioned this pull request Nov 23, 2023

Tracking issue for remote WAL based on Kafka #2722

Closed

8 tasks

chore: assign topics with hash map

6087d92

killme2008 requested review from MichaelScofield and v0y4g3r November 24, 2023 06:43

WenyXu reviewed Nov 24, 2023

View reviewed changes

src/meta-srv/src/metasrv/builder.rs Show resolved Hide resolved

src/meta-srv/src/metasrv/builder.rs Show resolved Hide resolved

niebayes mentioned this pull request Nov 28, 2023

feat(remote_wal): introduce region meta and integrate it into creating table metadata #2830

Closed

2 tasks

killme2008 reviewed Nov 28, 2023

View reviewed changes

niebayes marked this pull request as draft December 14, 2023 02:48

niebayes closed this Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(remote_wal): make meta srv able to alloc kafka topics #2753

feat(remote_wal): make meta srv able to alloc kafka topics #2753

niebayes commented Nov 15, 2023 •

edited

Loading

codecov bot commented Nov 16, 2023 •

edited

Loading

MichaelScofield left a comment

MichaelScofield Nov 16, 2023

niebayes Nov 23, 2023

niebayes Nov 23, 2023

niebayes commented Nov 24, 2023 •

edited

Loading

WenyXu left a comment

WenyXu Nov 24, 2023

killme2008 Nov 28, 2023

niebayes Dec 14, 2023

killme2008 commented Nov 30, 2023

feat(remote_wal): make meta srv able to alloc kafka topics #2753

feat(remote_wal): make meta srv able to alloc kafka topics #2753

Conversation

niebayes commented Nov 15, 2023 • edited Loading

What's changed and what's your intention?

Details

Checklist

Refer to a related PR or issue link (optional)

codecov bot commented Nov 16, 2023 • edited Loading

Codecov Report

MichaelScofield left a comment

Choose a reason for hiding this comment

MichaelScofield Nov 16, 2023

Choose a reason for hiding this comment

niebayes Nov 23, 2023

Choose a reason for hiding this comment

niebayes Nov 23, 2023

Choose a reason for hiding this comment

niebayes commented Nov 24, 2023 • edited Loading

WenyXu left a comment

Choose a reason for hiding this comment

WenyXu Nov 24, 2023

Choose a reason for hiding this comment

killme2008 Nov 28, 2023

Choose a reason for hiding this comment

niebayes Dec 14, 2023

Choose a reason for hiding this comment

killme2008 commented Nov 30, 2023

niebayes commented Nov 15, 2023 •

edited

Loading

codecov bot commented Nov 16, 2023 •

edited

Loading

niebayes commented Nov 24, 2023 •

edited

Loading