Infrastructure to meter updates by script for ra-s nontimeseries #108910
Conversation
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
server/src/main/java/org/elasticsearch/plugins/internal/DocumentSizeObserver.java
Outdated
Show resolved
Hide resolved
| @@ -109,8 +109,7 @@ public IndexResult index(Index index) throws IOException { | |||
| DocumentSizeAccumulator.EMPTY_INSTANCE | |||
| ); | |||
| ParsedDocument parsedDocument = index.parsedDoc(); | |||
| DocumentSizeObserver documentSizeObserver = parsedDocument.getDocumentSizeObserver(); | |||
| documentParsingReporter.onIndexingCompleted(documentSizeObserver); | |||
| documentParsingReporter.onIndexingCompleted(parsedDocument); | |||
There was a problem hiding this comment.
This is much cleaner, names makes more sense. I like it.
There was a problem hiding this comment.
Maybe we can find a better name for DocumentSizeObserver, (DocumentSizeAccumulator or similar?), but that's really a nitpick, and should be the last thing we do, when everything is in place.
| .setRefreshPolicy(request.getRefreshPolicy()) | ||
| .setNormalisedBytesParsed(documentSizeObserver.normalisedBytesParsed()); | ||
| .setRefreshPolicy(request.getRefreshPolicy()); | ||
| documentSizeObserver.setNormalisedBytesParsedOn(finalIndexRequest); |
There was a problem hiding this comment.
Nit: to be honest, I liked the old style more. But I'm not enough into the code (especially not in how this is used in serverless) to have a strong opinion.
There was a problem hiding this comment.
Same here, wondering if this is still necessary looking at the impls of setNormalisedBytesParsedOn.
Would below work (or are there cases where we don't want to set bytes parsed even if > 0)?
if(documentSizeObserver.normalisedBytesParsed() > 0) {
finalIndexRequest.setNormalisedBytesParsed(documentSizeObserver.normalisedBytesParsed());
}
There was a problem hiding this comment.
Oh I see, this is because we want someone to know not to set the normalized bytes, e.g. have an implementation that says public void setNormalisedBytesParsedOn(IndexRequest indexRequest) { /* do nothing */ }
Not loving the inversion, but I understand why now (as I said, not enough into code to understand how/if this could be better)
this commit refactors the metering for billing api so that we can hide the implementation details of DocumentSizeObserver creation and adds additional field
originatesFromScripton IndexRequestThere will no longer need to have a code checking if the request was already parsed in ingest service or updatehelper. This logic will be hidden in the implementation.