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

Handle SimpleAsyncTaskExecutor in WebSocketMessageBrokerStats #33104

Closed
csh0034 opened this issue Jun 27, 2024 · 5 comments
Closed

Handle SimpleAsyncTaskExecutor in WebSocketMessageBrokerStats #33104

csh0034 opened this issue Jun 27, 2024 · 5 comments
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Milestone

Comments

@csh0034
Copy link

csh0034 commented Jun 27, 2024

When using the following executor with virtual threads:

SimpleAsyncTaskExecutor executor = new SimpleAsyncTaskExecutor();
executor.setVirtualThreads(true);

The WebSocketMessageBrokerStats#getExecutorStatsInfo method returns "unknown".

private String getExecutorStatsInfo(@Nullable Executor executor) {
  if (executor == null) {
    return "null";
  }

  if (executor instanceof ThreadPoolTaskExecutor threadPoolTaskScheduler) {
    executor = threadPoolTaskScheduler.getThreadPoolExecutor();
  }

  if (executor instanceof ThreadPoolExecutor) {
    // It is assumed that the implementation of toString() in ThreadPoolExecutor
    // generates text that ends similar to the following:
    // pool size = #, active threads = #, queued tasks = #, completed tasks = #]
    String str = executor.toString();
    int indexOfPool = str.indexOf("pool");
    if (indexOfPool != -1) {
      // (length - 1) omits the trailing "]"
      return str.substring(indexOfPool, str.length() - 1);
    }
  }

  return "unknown";
}

It seems that only thread-pooling TaskExecutor was considered originally.
How about changing the wording?

Currently, it seems that there is no way to know if SimpleAsyncTaskExecutor is using virtual threads.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 27, 2024
@bclozel
Copy link
Member

bclozel commented Jun 27, 2024

What kind of statistics would you expect here?
It doesn't seem we can get anything like "pool size = #, active threads = #, queued tasks = #, completed tasks = #" in this case.

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: messaging Issues in messaging modules (jms, messaging) labels Jun 27, 2024
@csh0034
Copy link
Author

csh0034 commented Jun 28, 2024

Sorry, if the purpose is to expose pool information, then "unknown" is correct.

My intention was that after switching to using virtual threads, it showed "unknown" because virtual threads do not use pooling. It might be better to explicitly indicate that virtual threads are being used instead of showing "unknown".

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 28, 2024
@snicoll
Copy link
Member

snicoll commented Jul 4, 2024

Thanks for the feedback. I think you have a point it should indicate it's using virtual threads. However, AFAIK, we do not have a way to figure out that an Executor is using VT behind the scenes.

Paging @jhoeller for feedback to see if I missed it or if we could consider adding that.

@snicoll snicoll added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jul 4, 2024
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jul 9, 2024

Maybe what really matters is that it's not a pool based strategy rather than whether it's virtual threads or not. In Spring for GraphQL we have a check like this to determine whether the executor is pooling or not.

@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jul 12, 2024
@snicoll snicoll added this to the 6.2.x milestone Jul 12, 2024
@snicoll
Copy link
Member

snicoll commented Jul 12, 2024

The stats being about pooling information, we should indeed expose something like thread-per-task rather than unknown for this case.

@snicoll snicoll changed the title WebSocketMessageBrokerStats with VirtualThread Handle SimpleAsyncTaskExecutor in WebSocketMessageBrokerStats Jul 12, 2024
@rstoyanchev rstoyanchev self-assigned this Jul 12, 2024
@rstoyanchev rstoyanchev modified the milestones: 6.2.x, 6.2.0-M6 Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants