state store: getStream: log stream info#2758
Merged
hsaputra merged 2 commits intoapache:masterfrom Aug 18, 2021
Merged
Conversation
...mpl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/RootRangeStoreImpl.java
Outdated
Show resolved
Hide resolved
sursingh
reviewed
Jul 30, 2021
| final StreamProperties p = StreamProperties.parseFrom(streamPropBytes); | ||
| logger.info("namespace_id={} stream_id={} storage_container_id={} stream_name={} ", | ||
| nsId, p.getStreamId(), p.getStorageContainerId(), p.getStreamName()); | ||
| return FutureUtils.value(p); |
Contributor
There was a problem hiding this comment.
We should do this only when we create/delete/modify streams. Also some context in the log will be useful. eg "Created Stream" etc.
Contributor
Author
There was a problem hiding this comment.
moved logging to create and delete. add result of operation. since i'm only logging the (ns, id, name) tuple, i didn't add a log to modify for now.
Contributor
There was a problem hiding this comment.
Now that we have the log in create/delete, we should remove the log from here. This will create a unnecessary noise in logs
Contributor
Author
There was a problem hiding this comment.
oops. that was supposed to get removed. rebased.
mauricebarnum
force-pushed
the
log-stream-name
branch
2 times, most recently
from
08a6ee2 to
85dc033
Compare
logging (namespace id, stream id, stream name) tuple makes it easier to interpret the logs and stats expressed in terms of stream id. including the stream name in the stats would require a (cacheable) lookup in the root range store at runtime.
mauricebarnum
force-pushed
the
log-stream-name
branch
from
85dc033 to
4ef7ac0
Compare
hsaputra
reviewed
Aug 5, 2021
...mpl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/RootRangeStoreImpl.java
Show resolved
Hide resolved
ivankelly
approved these changes
Aug 17, 2021
Contributor
|
rerun failure checks |
Contributor
|
Will merge this by tomorrow if no more question or concern. Thanks |
Ghatage
pushed a commit
to sijie/bookkeeper
that referenced
this pull request
Jul 12, 2024
log the the tuple (namespace id, stream id, stream name) in RootStorageService getRange request. ### Motivation Server request metrics are labeled with the stream id, extracted from the routing header. The stream name (aka "table name") is not available but more useful. Rather than making a (cacheable) RPC request to fetch the id -> name mapping in the metrics, logging the information allows one to find the name without requiring admin access to the state store service. Reviewers: Ivan Kelly <ivank@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Henry Saputra <hsaputra@apache.org> This closes apache#2758 from mauricebarnum/log-stream-name and squashes the following commits: 4ef7ac0 [Maurice Barnum] cleanup: remove extraneous "final" declarations 284b643 [Maurice Barnum] state store: create and delete stream: log stream info
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
log the the tuple (namespace id, stream id, stream name) in RootStorageService getRange request.
Motivation
Server request metrics are labeled with the stream id, extracted from the routing header. The stream name
(aka "table name") is not available but more useful. Rather than making a (cacheable) RPC request to fetch
the id -> name mapping in the metrics, logging the information allows one to find the name without requiring
admin access to the state store service.