bruno-roustant commented on a change in pull request #1105: LUCENE-9105: UniformSplit postings format detects corrupted index URL: https://github.com/apache/lucene-solr/pull/1105#discussion_r361119723
########## File path: lucene/codecs/src/java/org/apache/lucene/codecs/uniformsplit/IndexDictionary.java ########## @@ -97,31 +100,47 @@ * Otherwise {@code -1} if there is no floor block key because the searched * term precedes alphabetically the first block key of the dictionary. */ - long seekBlock(BytesRef term); + long seekBlock(BytesRef term) throws IOException; /** * Returns the next block key and positions the browser at this key. * A key is a prefix of a term in the dictionary. * If seekBlock was just called then this is the current block key. */ - BytesRef nextKey(); + BytesRef nextKey() throws IOException; /** * Returns the next key without advancing. * Only call this after {@link #nextKey()} returns a non-null result. */ - BytesRef peekKey(); + BytesRef peekKey() throws IOException; /** * Returns the number of characters of this block's key that is in common with all terms in this block. * Only call this after {@link #nextKey()} returns a non-null result. */ - int getBlockPrefixLen(); + int getBlockPrefixLen() throws IOException; /** * Returns the block file pointer associated with the key returned. * Only call this after {@link #nextKey()} returns a non-null result. */ - long getBlockFilePointer(); + long getBlockFilePointer() throws IOException; + } + + /** + * Supplier for a new stateful {@link Browser} created on the immutable {@link IndexDictionary}. + * <p> + * The immutable {@link IndexDictionary} is lazy loaded thread safely. This lazy loading allows + * us to load it only when {@link org.apache.lucene.index.TermsEnum#seekCeil} or + * {@link org.apache.lucene.index.TermsEnum#seekExact} are called (it is not loaded for a direct + * all-terms enumeration). + */ + interface BrowserSupplier extends Accountable { Review comment: Thanks. I looked externally for such supplier but I didn't think of looking in Lucene util... Accountable is leveraged in UniformSplitTerms.getDictionaryRamBytesUsed() when computing the heap space. We could indeed put there an instanceof check and optionally cast to Accountable. But I'd prefer to mandate Accountable so that any implementation doesn't forget to provide it. What about keeping BrowserSupplier to join IOSupplier and Accountable (without additional interface contract)? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org