You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "vrajat (via GitHub)" <gi...@apache.org> on 2025/03/10 12:58:08 UTC
[I] Tracing::ThreadUsageResourceAccountant cannot be isolated and tested in integration tests [pinot]
vrajat opened a new issue, #15231:
URL: https://github.com/apache/pinot/issues/15231
In integration tests, brokers and servers run in the same JVM. All of them share the same `ThreadUsageResourceAccountant` in a singleton in `Tracing.java`. This makes certain kinds of tests hard to write. For example:
* #14999
* Tests that involve relevant operations in a specific server.
Some tests use `InterruptingAccountant`. This class is useful to inject errors in the broker. However, it is not possible to inject errors in servers as the broker will error out first.
Couple of options are:
* Run a server in a different JVM. This is a major change in how tests have to be written. It will also make debug mode harder.
* Use dependency injection rather than a singleton to pass around an accountant object.
--
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: commits-unsubscribe@pinot.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [I] Tracing::ThreadUsageResourceAccountant cannot be isolated and tested in integration tests [pinot]
Posted by "vrajat (via GitHub)" <gi...@apache.org>.
vrajat commented on issue #15231:
URL: https://github.com/apache/pinot/issues/15231#issuecomment-2954641545
In PerQueryCpuMemAccountant, there is a
```
// the map to track stats entry for each thread, the entry will automatically be added when one calls
// setThreadResourceUsageProvider on the thread, including but not limited to
// server worker thread, runner thread, broker jetty thread, or broker netty thread
private final ConcurrentHashMap<Thread, CPUMemThreadLevelAccountingObjects.ThreadEntry> _threadEntriesMap
= new ConcurrentHashMap<>();
```
Entries are automatically added, when the following thread local is first accessed:
```
private final ThreadLocal<CPUMemThreadLevelAccountingObjects.ThreadEntry> _threadLocalEntry
= ThreadLocal.withInitial(() -> {
CPUMemThreadLevelAccountingObjects.ThreadEntry ret =
new CPUMemThreadLevelAccountingObjects.ThreadEntry();
_threadEntriesMap.put(Thread.currentThread(), ret);
LOGGER.info("Adding thread to _threadLocalEntry: {}", Thread.currentThread().getName());
return ret;
}
);
```
To the best of my knowledge, the first access is always a call to setupRunner or setupWorker. For example:
```
@Override
public void setupRunner(@Nullable String queryId, int taskId, ThreadExecutionContext.TaskType taskType) {
_threadLocalEntry.get()._errorStatus.set(null);
if (queryId != null) {
_threadLocalEntry.get()
.setThreadTaskStatus(queryId, CommonConstants.Accounting.ANCHOR_TASK_ID, taskType, Thread.currentThread());
}
}
```
These calls are made in specific places such as in OpChainSchedulerService like
```
try (OpChain closeMe = operatorChain) {
Tracing.ThreadAccountantOps.setupWorker(operatorChain.getId().getStageId(),
ThreadExecutionContext.TaskType.MSE, operatorChain.getParentContext());
LOGGER.trace("({}): Executing", operatorChain);
```
My opinion is that that there is no advantage to automatically add thread entries to `_threadEntriesMap`. A developer still has to put the effort to instrument the code with `setupRunner` and `setupWorker`. Instead, the code can add an entry to a thread entries map in `OpChainSchedulerService` itself. This is a concurrent map initialized in `BaseServerStarter` and injected during construction. I dont see the point in effect having a global `_threadEntriesMap`.
A similar argument can be made for the infra in the broker as well.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [I] Tracing::ThreadUsageResourceAccountant cannot be isolated and tested in integration tests [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz closed issue #15231: Tracing::ThreadUsageResourceAccountant cannot be isolated and tested in integration tests
URL: https://github.com/apache/pinot/issues/15231
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [I] Tracing::ThreadUsageResourceAccountant cannot be isolated and tested in integration tests [pinot]
Posted by "vrajat (via GitHub)" <gi...@apache.org>.
vrajat commented on issue #15231:
URL: https://github.com/apache/pinot/issues/15231#issuecomment-2710507035
cc @jasperjiaguo @vvivekiyer
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org