DMA reads #12
Open
DMA reads #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jul-stas
reviewed
Apr 26, 2022
jul-stas
reviewed
Apr 26, 2022
|
|
||
| void opentelemetry_state_data::serialize_dma_counter(bytes& serialized) const { | ||
| const auto counter = htonl(_dma_counter); | ||
| const auto *counter_ptr = reinterpret_cast<const int8_t*>(&counter); |
There was a problem hiding this comment.
By chance, int8_t* doesn't break strict aliasing?
jul-stas
reviewed
Apr 26, 2022
service/storage_proxy.cc
Outdated
| return _proxy->_messaging.send_read_data(netw::messaging_service::msg_addr{ep, 0}, timeout, *_cmd, _partition_range, opts.digest_algo).then([this, ep](rpc::tuple<query::result, rpc::optional<cache_temperature>> result_hit_rate) { | ||
| auto&& [result, hit_rate] = result_hit_rate; | ||
| return _proxy->_messaging.send_read_data(netw::messaging_service::msg_addr{ep, 0}, timeout, *_cmd, _partition_range, opts.digest_algo).then([this, ep](rpc::tuple<query::result, rpc::optional<cache_temperature>, tracing::trace_state_ptr::cache_counter_t, tracing::trace_state_ptr::dma_counter_t, tracing::trace_state_ptr::dma_size_t> result_hit_rate) { | ||
| auto&& [result, hit_rate, cache_counter, dma_counter, dma_size] = result_hit_rate; |
There was a problem hiding this comment.
(I suggest that we put comments wrt. "parent PRs" here, so they can be all fixed at once)
My suggestion is to modernize the name of result_hit_rate
jul-stas
reviewed
Apr 26, 2022
smaczliwka
force-pushed
the
opentelemetry-dma-reads
branch
2 times, most recently
from
f6f9e1f to
f912bab
Compare
Add an attribute to opentelemetry_state to store a number of partitions read from the cache. Extend RPC READ_DATA procedure to pass a number of reads from the cache in replicas. It's summed up by the coordinator and returned in custom_payload.
smaczliwka
force-pushed
the
opentelemetry-dma-reads
branch
from
f912bab to
10e6316
Compare
margdoc
force-pushed
the
opentelemetry-dma-reads
branch
from
10e6316 to
1ee37d7
Compare
Move gathered telemetry data to opentelemetry_state_data and make it sharded.
margdoc
force-pushed
the
opentelemetry-dma-reads
branch
from
1ee37d7 to
4ac7e6a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PR introduces new replica-side tracing information gathered in opentelemetry state and send by a coordinator to the client in custom payload: