[GitHub] [lucene] vigyasharma commented on pull request #1057: LUCENE-10670: Add a codec class to track merge time of each index part

[GitBox <gi...@apache.org>]

vigyasharma commented on PR #1057:
URL: https://github.com/apache/lucene/pull/1057#issuecomment-1224694259

   > it uses `instanceof`: https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsWriter.java#L686
   
   Thanks for sharing that link @rmuir. Intrigued, I did a quick search for `instanceof` in the `codecs/`, and found such checks in more than one place. 
   
   It seems that when we add a functionality in a new codec version, we use such type equality checks to ensure that consuming code is compatible. Adding a generic codec wrapper that branches from the inheritance chain will break such checks. Replacing such checks is a whole other discussion and not really the problem we are solving here.
   
   I like the idea of making `SegmentMerger` implement some sort of a Stats interface that can return a metrics object. We can populate the TimeMetric object in `SegmentMerger.merge()` as each codec's merge returns. It solves what is needed here, and is easy to extend.


-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org