Skip to content

feat(interactive): Impl kafka wal writer and wal paresr #4518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

zhanglei1949
Copy link
Collaborator

as titled.

@zhanglei1949 zhanglei1949 marked this pull request as draft February 24, 2025 06:48
Copy link
Contributor

github-actions bot commented Feb 24, 2025

Please check the preview of the documentation changes at
https://db2e6970.graphscope-docs-preview.pages.dev

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.76%. Comparing base (1fc617f) to head (2f15484).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #4518       +/-   ##
===========================================
+ Coverage   33.02%   44.76%   +11.73%     
===========================================
  Files         127       12      -115     
  Lines       13299      592    -12707     
===========================================
- Hits         4392      265     -4127     
+ Misses       8907      327     -8580     

see 127 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fc617f...2f15484. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…baba#4517)

Fixes alibaba#4454

---------

Signed-off-by: siyuan0322 <[email protected]>

fix aocc

Committed-by: [email protected] from Dev container

fix installation

Committed-by: [email protected] from Dev container

fix linking,todo: verify correctness

Committed-by: xiaolei.zl from Dev container

impl kafka writer and parser

Committed-by: xiaolei.zl from Dev container

Committed-by: xiaolei.zl from Dev container

Committed-by: xiaolei.zl from Dev container

minor fix

Committed-by: xiaolei.zl from Dev container

Committed-by: xiaolei.zl from Dev container

Committed-by: xiaolei.zl from Dev container

fix

Committed-by: xiaolei.zl from Dev container

Committed-by: xiaolei.zl from Dev container
@liulx20 liulx20 force-pushed the impl-kafka-writer branch from 3c53187 to 56d4fc5 Compare March 25, 2025 09:42
@liulx20 liulx20 force-pushed the impl-kafka-writer branch 5 times, most recently from f117236 to a7e1487 Compare March 26, 2025 07:04
@liulx20 liulx20 force-pushed the impl-kafka-writer branch from a7e1487 to 6f32349 Compare March 26, 2025 07:11
@zhanglei1949 zhanglei1949 marked this pull request as ready for review March 27, 2025 01:10
liulx20
liulx20 previously approved these changes Mar 27, 2025
@liulx20 liulx20 requested a review from Copilot March 28, 2025 05:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for persisting the Write-Ahead Log (WAL) to Kafka along with a Kafka WAL parser, and updates the testing workflows accordingly.

  • Added documentation for setting up and testing Kafka-based WAL persistence.
  • Updated CI workflow to include steps for building and executing Kafka WAL writer/parser tests.

Reviewed Changes

Copilot reviewed 7 out of 25 changed files in this pull request and generated 2 comments.

File Description
docs/flex/interactive/development/dev_and_test.md Added instructions on installing Kafka and testing the Kafka WAL writer/parser
.github/workflows/interactive.yml Added a new workflow step to build, run, and test Kafka WAL writer/parser functionality
Files not reviewed (18)
  • flex/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.cc: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.h: Language not supported
  • flex/engines/graph_db/database/graph_db.cc: Language not supported
  • flex/engines/graph_db/database/graph_db.h: Language not supported
  • flex/engines/graph_db/database/graph_db_session.cc: Language not supported
  • flex/engines/graph_db/database/graph_db_session.h: Language not supported
  • flex/engines/graph_db/database/insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_edge_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_vertex_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/update_transaction.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_writer.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_writer.h: Language not supported
Comments suppressed due to low confidence (1)

.github/workflows/interactive.yml:382

  • Consider adding a wait command (e.g., a sleep or a health check) after starting the Kafka server to ensure it is fully initialized before proceeding with topic creation and tests.
bin/kafka-server-start.sh config/kraft/reconfig-server.properties &

@liulx20 liulx20 self-requested a review March 28, 2025 06:11
liulx20
liulx20 previously approved these changes Mar 28, 2025
@liulx20 liulx20 force-pushed the impl-kafka-writer branch 5 times, most recently from ac495be to fa7423e Compare March 28, 2025 12:11
@liulx20 liulx20 requested review from Copilot and liulx20 March 31, 2025 03:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for Kafka-based WAL writing and parsing in the interactive component, along with updated documentation and test workflows.

  • Added documentation for installing and running Kafka for WAL persistence.
  • Extended the GitHub workflow to run Kafka WAL tests.

Reviewed Changes

Copilot reviewed 14 out of 32 changed files in this pull request and generated 1 comment.

File Description
docs/flex/interactive/development/dev_and_test.md Added Kafka WAL documentation and installation/test instructions.
.github/workflows/interactive.yml Introduced a new workflow step to run and test Kafka WAL writer/parser.
Files not reviewed (18)
  • flex/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.cc: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.h: Language not supported
  • flex/engines/graph_db/database/graph_db.cc: Language not supported
  • flex/engines/graph_db/database/graph_db.h: Language not supported
  • flex/engines/graph_db/database/graph_db_session.cc: Language not supported
  • flex/engines/graph_db/database/graph_db_session.h: Language not supported
  • flex/engines/graph_db/database/insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_edge_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_vertex_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/update_transaction.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_utils.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_utils.h: Language not supported
Comments suppressed due to low confidence (1)

.github/workflows/interactive.yml:385

  • The Kafka test command uses port 9092, while the documentation specifies port 902. Please ensure that a consistent Kafka port is used across tests.
../tests/hqps/kafka_test localhost:9092 kafka-test

Comment on lines +390 to +391
ps aux | grep kafka | grep -v grep | awk '{print $2}' | xargs kill -9

Copy link
Preview

Copilot AI Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The process termination command may kill unrelated processes. Consider refining the filter to specifically target the Kafka process started by the workflow.

Suggested change
ps aux | grep kafka | grep -v grep | awk '{print $2}' | xargs kill -9
kill -9 $(cat kafka_pid.txt)

Copilot uses AI. Check for mistakes.

@liulx20 liulx20 force-pushed the impl-kafka-writer branch from fa7423e to a3e3338 Compare March 31, 2025 03:38
@liulx20 liulx20 requested a review from Copilot March 31, 2025 06:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for persisting the Write-Ahead Log (WAL) to Kafka and integrates corresponding tests into the CI workflow.

  • Adds documentation and command instructions for setting up and testing Kafka-based WAL persistence.
  • Introduces a new GitHub Actions job to build and run tests for KafkaWalWriter and KafkaWalParser.

Reviewed Changes

Copilot reviewed 14 out of 32 changed files in this pull request and generated 1 comment.

File Description
docs/flex/interactive/development/dev_and_test.md Adds a new section with setup and test instructions for Kafka WAL persistence.
.github/workflows/interactive.yml Introduces a new CI job to test Kafka WAL functionality, including building, executing tests, and cleaning up Kafka topics.
Files not reviewed (18)
  • flex/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/CMakeLists.txt: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.cc: Language not supported
  • flex/engines/graph_db/app/kafka_wal_ingester_app.h: Language not supported
  • flex/engines/graph_db/database/graph_db.cc: Language not supported
  • flex/engines/graph_db/database/graph_db.h: Language not supported
  • flex/engines/graph_db/database/graph_db_session.cc: Language not supported
  • flex/engines/graph_db/database/graph_db_session.h: Language not supported
  • flex/engines/graph_db/database/insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_edge_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/single_vertex_insert_transaction.cc: Language not supported
  • flex/engines/graph_db/database/update_transaction.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.cc: Language not supported
  • flex/engines/graph_db/database/version_manager.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_parser.h: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_utils.cc: Language not supported
  • flex/engines/graph_db/database/wal/kafka_wal_utils.h: Language not supported
Comments suppressed due to low confidence (1)

docs/flex/interactive/development/dev_and_test.md:293

  • There appears to be a potential discrepancy in the Kafka test port: the documentation test command uses port 'localhost:902' while the CI workflow uses 'localhost:9092'. Confirm and align the intended Kafka server port in documentation and tests.
### Test KafkaWalWriter and KafkaWalParser

Comment on lines +382 to +383
bin/kafka-server-start.sh config/kraft/reconfig-server.properties &

Copy link
Preview

Copilot AI Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a delay (or a readiness check) after starting the Kafka server to ensure it is fully initialized before subsequent commands are executed.

Suggested change
bin/kafka-server-start.sh config/kraft/reconfig-server.properties &
bin/kafka-server-start.sh config/kraft/reconfig-server.properties &
# Wait for Kafka server to be ready
while ! bin/kafka-topics.sh --list --bootstrap-server localhost:9092; do
echo "Waiting for Kafka to be ready..."
sleep 5
done

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants