Skip to content
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

Refactoring #2

Merged
merged 15 commits into from
May 20, 2023
Merged

Refactoring #2

merged 15 commits into from
May 20, 2023

Conversation

kassane
Copy link
Owner

@kassane kassane commented May 18, 2023

First experiment, turning very complicated and inefficient.
Currently refactored and tested in CI/CD together with tigerbeetle server.

Use of classes and (smart-)pointers will be minimized only in specific use cases, like RAII.

cc: @batiati

@kassane
Copy link
Owner Author

kassane commented May 18, 2023

https://github.com/kassane/tb-client-cpp/actions/runs/5014156725/jobs/8988076462#step:7:16

zig 0.9.1(clang-13) no has c++20 full-support. Works in zig 0.11.0-dev (clang-16)

@kassane
Copy link
Owner Author

kassane commented May 18, 2023

@batiati, the c client is missing some step during the run?

Currently in the CI (C++ sample) maybe there is a issue with the std::condition_variable and std::mutex, plus the account size(array) + sizeof(tb_account_t)

C++ sample - output
See: https://github.com/kassane/tb-client-cpp/actions/runs/5015665906/jobs/8991522383?pr=2#step:8:1

C sample - output
$> /zig-out/bin/c_sample 
TigerBeetle C Sample
Connecting...
debug(tb_client_context): 73372712115674697003588304400769413876: init: initializing
debug(tb_client_context): 73372712115674697003588304400769413876: init: allocating tb_packets
debug(tb_client_context): 73372712115674697003588304400769413876: init: parsing vsr addresses: 127.0.0.1:3000
debug(tb_client_context): 73372712115674697003588304400769413876: init: initializing IO
debug(tb_client_context): 73372712115674697003588304400769413876: init: initializing MessagePool
debug(tb_client_context): 73372712115674697003588304400769413876: init: initializing client (cluster_id=0, addresses={ 127.0.0.1:3000 })
debug(vsr): 73372712115674697003588304400769413876: ping_timeout started
debug(tb_client_context): 73372712115674697003588304400769413876: init: initializing thread
debug(tb_client_thread): 73372712115674697003588304400769413876: init: initializing signal
debug(tb_client_thread): 73372712115674697003588304400769413876: init: spawning thread
Creating accounts...
debug(message_bus): connecting to replica 0 in 94ms...
debug(client): 73372712115674697003588304400769413876: register: registering a session with the cluster
debug(client): 73372712115674697003588304400769413876: send_request_for_the_first_time: request=0 checksum=155301856535823733174576865266543992954
debug(vsr): 73372712115674697003588304400769413876: request_timeout started
debug(client): 73372712115674697003588304400769413876: sending request to replica 0: Header{ .checksum = 155301856535823733174576865266543992954, .checksum_body = 98287347720187652707502696638535748739, .parent = 0, .client = 73372712115674697003588304400769413876, .context = 0, .request = 0, .cluster = 0, .epoch = 0, .view = 0, .op = 0, .commit = 0, .timestamp = 0, .size = 128, .replica = 0, .command = Command.request, .operation = Operation.register, .version = 0 }
debug(client): 73372712115674697003588304400769413876: request: user_data=541594930264586538710988464 request=1 size=384 create_accounts
debug(message_bus): connecting to replica 0...
error(message_bus): error connecting to replica 0: error.ConnectionRefused
debug(message_bus): connecting to replica 0 in 105ms...
debug(message_bus): connecting to replica 0...
error(message_bus): error connecting to replica 0: error.ConnectionRefused
debug(message_bus): connecting to replica 0 in 90ms...
debug(message_bus): connecting to replica 0...
error(message_bus): error connecting to replica 0: error.ConnectionRefused
debug(message_bus): connecting to replica 0 in 274ms...
debug(message_bus): connecting to replica 0...
error(message_bus): error connecting to replica 0: error.ConnectionRefused
debug(message_bus): connecting to replica 0 in 732ms...

@batiati
Copy link

batiati commented May 18, 2023

@batiati, the c client is missing some step during the run?

It looks like there isn't a TigerBeetle server available to connect to, as the client keeps trying to reconnect.
Also, the cpp client run looks suspect due to the huge throughput and message "No accounts found!" at the end 🤔

@kassane
Copy link
Owner Author

kassane commented May 18, 2023

It looks like there isn't a TigerBeetle server available to connect to, as the client keeps trying to reconnect.

So I'll need try to run the server in the background. 🫣

Also, the cpp client run looks suspect due to the huge throughput and message "No accounts found!" at the end 🤔

I suspect that there are problems with mutex, which by the way in C++ it is common to assign un/lock(RAII) when using std::lock_guard<std::mutex>(ctx->mutex);
When I restored posix API (C sample) in C++ it returned the same result as in C. 😔

@batiati
Copy link

batiati commented May 18, 2023

So I'll need try to run the server in the background. 🫣

There's a helper for that in TigerBeetle's build.zig: run_with_tb
https://github.com/tigerbeetledb/tigerbeetle/blob/main/src/clients/README.md#run_with_tbzig--run_with_tb

@kassane
Copy link
Owner Author

kassane commented May 19, 2023

I suspect that there are problems with mutex, which by the way in C++ it is common to assign un/lock(RAII) when using std::lock_guard<std::mutex>(ctx->mutex); When I restored posix API (C sample) in C++ it returned the same result as in C. pensive

This solve issue. Sync sample with thread-safe tb_client_submit:
https://github.com/kassane/tb-client-cpp/blob/fd8bb8347b0033c6207701ffcf6ea0ffc8728719/include/tb_client.hpp#L86-L94

There's a helper for that in TigerBeetle's build.zig: run_with_tb

This latest commit adds CMake full-support for tigerbeetle. Please give it a read to improve it, I'd appreciate it.
Tigerbeetle with cmake-support for C and C++ project.

@batiati
Copy link

batiati commented May 19, 2023

This solve issue. Sync sample with thread-safe tb_client_submit

Yes, that's the idea when converting the async API from tb_client to a traditional blocking API. Instead of a spin wait like this, you can use a mutex+condvar to signal the completion, saving a lot of CPU.

I think C++ could follow the same strategy we have with Java/C# clients, exposing both async and blocking APIs.
An idiomatic C++ API could provide an async API by exposing a high-level callback/lambda, or even explore modern C++ async (is that a use case for promises?).

@kassane
Copy link
Owner Author

kassane commented May 19, 2023

I think C++ could follow the same strategy we have with Java/C# clients, exposing both async and blocking APIs.
An idiomatic C++ API could provide an async API by exposing a high-level callback/lambda, or even explore modern C++ async (is that a use case for promises?).

Yes, it could be enhanced to work asynchronously.
However, C++'s stdlib doesn't have a true std::async. It depends on third party resources to improve it.
ref: cpp-asio - executores (ptBR )
I will open a case study at this point.

Now that the initial example is working, I want to improve the CMake support.

@kassane kassane merged commit 59b3957 into main May 20, 2023
3 checks passed
@kassane kassane deleted the refactoring branch May 20, 2023 15:31
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.

None yet

2 participants