You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/04/18 02:54:31 UTC
[GitHub] [skywalking] yangyiweigege opened a new issue, #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
yangyiweigege opened a new issue, #8897:
URL: https://github.com/apache/skywalking/issues/8897
### Search before asking
- [X] I had searched in the [issues](https://github.com/apache/skywalking/issues?q=is%3Aissue) and found no similar issues.
### Apache SkyWalking Component
OAP server (apache/skywalking)
### What happened
org.apache.skywalking.oap.server.core.storage.PersistenceTimer#extractDataAndSave:
will initialize BulkProcessor multiple times in concurrent and create multiple flush task, in actually, BatchProcessEsDAO have only one referenced BulkProcessor.
### What you expected to happen
BatchProcessEsDAO only init BulkProcessor once time.
### How to reproduce
in concurrent init.
### Anything else
you can see at last one flush schedule :
<img width="1036" alt="6592F812-C690-479B-85EA-E36EE03EA8CA" src="https://user-images.githubusercontent.com/23202824/163746291-27d1e44a-2e24-410a-bc0b-7ba321266bf0.png">
but one of the flush task,the request is all zero:
<img width="865" alt="image" src="https://user-images.githubusercontent.com/23202824/163746360-4d2ba68b-e935-425b-897f-48433f1dc31d.png">
no actually meaning.
how to avoid:
the init code in org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base.BatchProcessEsDAO#flush by DCL.
```java
if (bulkProcessor == null) {
synchronized(this) {
bulkProcessor = new BulkProcessor();
}
}
```
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
--
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: notifications-unsubscribe@skywalking.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101254988
Yes, agree.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101235290
Agree, I can't see the case, that is why I am just pending on what he is going to propose. No data to consumer is common, such as no log reported.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] yangyiweigege commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
yangyiweigege commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101245769
code in org.apache.skywalking.oap.server.core.storage.PersistenceTimer#extractDataAndSave:
```java
final CompletableFuture<Void> future =
CompletableFuture.allOf(workers.stream().map(worker -> {
return CompletableFuture.runAsync(() -> {
List<PrepareRequest> innerPrepareRequests;
// Prepare stage
try (HistogramMetrics.Timer ignored = prepareLatency.createTimer()) {
if (log.isDebugEnabled()) {
log.debug(
"extract {} worker data and save",
worker.getClass().getName()
);
}
innerPrepareRequests = worker.buildBatchRequests();
worker.endOfRound();
}
if (CollectionUtils.isEmpty(innerPrepareRequests)) {
return;
}
// Execution stage
HistogramMetrics.Timer executeLatencyTimer = executeLatency.createTimer();
batchDAO.flush(innerPrepareRequests)
.whenComplete(($1, $2) -> executeLatencyTimer.close());
}, prepareExecutorService);
}).toArray(CompletableFuture[]::new));
```
batchDAO is only one, but works have many and each work will invoke batchDAO.flush in different threads, and batchDAO only have one BulkProcessor. it means:
if (BulkProcessor == null ) can invoked many time。
batchDAO have only one BulkProcessor.
Multiple scheduled tasks are initialized, but in reality, only one is useful. For the rest， ArrayBlockingQueue<Holder> requests always 0 size.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101249128
OK, yes, it could be in a concurrency case. @yangyiweigege You are correct, we miss a lock here.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
URL: https://github.com/apache/skywalking/issues/8897
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101063891
Assigned to you. Wait for your PR to discuss details on that.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101229191
I don’t think there is “concurrent init” as you said. Please read my comment in the other issue you posted. The fact you observed is true but the analysis is wrong.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] yangyiweigege commented on issue #8897: [Bug] ES BulkProcessor in BatchProcessEsDAO Initialize multiple times
Posted by GitBox <gi...@apache.org>.
yangyiweigege commented on issue #8897:
URL: https://github.com/apache/skywalking/issues/8897#issuecomment-1101254240
> OK, yes, it could be in a concurrency case. @yangyiweigege You are correct, we miss a lock here.
i think can use double-checked locking(DCL) in (BatchProcessEsDAO.flush and insert ) to avoid the bulkProcessor init many times.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org