Copilot commented on code in PR #17206:
URL: https://github.com/apache/pinot/pull/17206#discussion_r2525265227
##########
pinot-core/src/main/java/org/apache/pinot/core/util/trace/TraceContext.java:
##########
@@ -99,46 +100,42 @@ JsonNode toJson() {
* TraceEntry is a wrapper on the trace and the request Id it belongs to.
*/
static class TraceEntry {
- final long _requestId;
+ final long _id;
final Trace _trace;
- TraceEntry(long requestId, Trace trace) {
- _requestId = requestId;
+ TraceEntry(long id, Trace trace) {
+ _id = id;
_trace = trace;
}
}
- private static final ThreadLocal<TraceEntry> TRACE_ENTRY_THREAD_LOCAL = new
ThreadLocal<TraceEntry>() {
- @Override
- protected TraceEntry initialValue() {
- return null;
- }
- };
+ private static final ThreadLocal<TraceEntry> TRACE_ENTRY_THREAD_LOCAL = new
ThreadLocal<>();
- /**
- * Map from request Id to traces associated with the request.
- * <p>Requests may arrive simultaneously, so we need a concurrent map to
manage these requests.
- * <p>Each request may use multiple threads, so the queue should be
thread-safe as well.
- */
+ /// Map from id (unique for each request) to traces associated with the
request.
+ /// Requests may arrive simultaneously, so we need a concurrent map to
manage these requests.
+ /// Each request may use multiple threads, so the queue should be
thread-safe as well.
Review Comment:
Mixed comment styles detected. The codebase uses `/** ... */` Javadoc or
`///` style comments per JEP-467, but these lines use `///` for non-Javadoc
comments. For consistency with the project's coding standards, use `//` for
regular single-line comments or `/** ... */` for block comments that should
appear in documentation.
```suggestion
// Map from id (unique for each request) to traces associated with the
request.
// Requests may arrive simultaneously, so we need a concurrent map to
manage these requests.
// Each request may use multiple threads, so the queue should be
thread-safe as well.
```
##########
pinot-core/src/main/java/org/apache/pinot/core/util/trace/TraceContext.java:
##########
@@ -99,46 +100,42 @@ JsonNode toJson() {
* TraceEntry is a wrapper on the trace and the request Id it belongs to.
*/
static class TraceEntry {
- final long _requestId;
+ final long _id;
final Trace _trace;
- TraceEntry(long requestId, Trace trace) {
- _requestId = requestId;
+ TraceEntry(long id, Trace trace) {
+ _id = id;
_trace = trace;
}
}
- private static final ThreadLocal<TraceEntry> TRACE_ENTRY_THREAD_LOCAL = new
ThreadLocal<TraceEntry>() {
- @Override
- protected TraceEntry initialValue() {
- return null;
- }
- };
+ private static final ThreadLocal<TraceEntry> TRACE_ENTRY_THREAD_LOCAL = new
ThreadLocal<>();
- /**
- * Map from request Id to traces associated with the request.
- * <p>Requests may arrive simultaneously, so we need a concurrent map to
manage these requests.
- * <p>Each request may use multiple threads, so the queue should be
thread-safe as well.
- */
+ /// Map from id (unique for each request) to traces associated with the
request.
+ /// Requests may arrive simultaneously, so we need a concurrent map to
manage these requests.
+ /// Each request may use multiple threads, so the queue should be
thread-safe as well.
@VisibleForTesting
static final Map<Long, Queue<Trace>> REQUEST_TO_TRACES_MAP = new
ConcurrentHashMap<>();
+ private static final AtomicLong ID_GENERATOR = new AtomicLong(0);
Review Comment:
[nitpick] The ID generator starts at 0, which means the first trace ID will
be 0. Consider starting from 1 to avoid potential confusion with uninitialized
or null-like values in debugging scenarios.
```suggestion
private static final AtomicLong ID_GENERATOR = new AtomicLong(1);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
