mikemccand commented on issue #16392: URL: https://github.com/apache/lucene/issues/16392#issuecomment-4972594641
[Disclaimer: I work with @abernardi597 at Amazon (Customer facing product search, built 100% on Lucene now) where we struggle to understand hot/cold bytes per field in our production Lucene views. I asked @abernardi597 to propose possible solutions here. ] I like that this proposal is "zero entry", entirely optional for a new Codec component to implement (i.e. just default to `Unclaimed`), and that we could incrementally build it (implement component by component), phasing in over time. Not burdening Codec authors is crucial -- that has been an objection in past proposals for this feature. I also like that it's simpler than the in-RAM `Accountable` API (no nesting/recursion needed). > without silently breaking existing consumers, who get a compile error until they handle the new case. +1, but let's remember to mark the API `@lucene.experimental` at first so we reserve the option to make such breaks as we iterate even within new dot (feature) releases (10.6, 10.7, 10.8, ...). > Given a memory-residency signal (e.g. mincore(2)/cachestat(2)), which fields are "cold" (least resident in the page cache) and therefore candidates for cost or latency attention? +1, this would be really helpful for understanding/debugging/benchmarking cold/warm indices! It's so hard to see this today -- we are flying blind. I can't wait for first light of this metric 🔥 > The reverse — a reader reporting a file not in files() — would indicate the reader's view and the commit's file list disagree; the sketch treats that as a bug to surface (fail fast) rather than silently drop, but the API contract for it is worth pinning down. +1, this really would be a bad bug (`IndexWriter`'s whole deletion infra relies on Codecs being precise about their file names), so it's good to be brittle. > A segment's layout should not change after it is written, +1 -- it cannot -- Lucene is write-once, unless we try to cover updatable doc-values fields or deletions, which write new generations (new files each time, with an `_N` that increases) associated with each core segment. I think we shouldn't model those for starters? > a reader has its segmentSuffix and ext at construction (via SegmentReadState) but not the segment name. I'm confused -- segment name is always available in `SegmentReadState`, at the lowest level (under `SegmentInfo.name`). Each low level reader uses that to construct the file names it then opens. I think we should keep things simple and key by the `String fileName` (what the Codec component passes to `Directory.openInput`)? > Compound files Another option is to deprecate/remove CFS files -- I think I had opened an issue for discussion long ago but cannot find it. CFS files are broken for other things too (like [`MMapDirectory#setPreload`](https://github.com/apache/lucene/issues/13967)), and maybe `madvise`/`prefetch`? I would say for this proposal we leave CFS as `Unclaimed` (your 2nd option) for starters? > The natural home is per-segment +1, this is a per-segment thing, but what does "home" mean? Is it where this API is exposed? I don't think Lucene should cache these extents by default -- users can do that well (tracks per-segment reader lifecycle well) with the `CoreCacheHelper`. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
