github-actions[bot] commented on code in PR #70262:
URL: https://github.com/apache/doris/pull/70262#discussion_r4500047027


##########
be/src/storage/index/inverted/analysis/analyzer/analyzer.cpp:
##########
@@ -43,25 +43,106 @@
 #include "storage/index/inverted/analysis/analyzer/kuromoji/KuromojiAnalyzer.h"
 #include 
"storage/index/inverted/analysis/analyzer/kuromoji/kuromoji_normalize.h"
 #include 
"storage/index/inverted/analysis/char_filter/char_replace_char_filter_factory.h"
+#include "storage/index/inverted/analysis/gram/gram_scheme.h"
 #include "storage/index/inverted/analysis/token_filter/lower_case_filter.h"
+#include "storage/index/inverted/clucene/lucene_backed_text_analyzer.h"
 #include "util/string_util.h"
 
 namespace doris::segment_v2::inverted_index {
 namespace {
 
+// The parser of a configuration whose analyzer is builtin, named by the 
parser or by the analyzer.
+InvertedIndexParserType builtin_parser_type(const InvertedIndexAnalyzerConfig& 
config) {
+    return config.analyzer_name.empty()
+                   ? config.parser_type
+                   : 
get_inverted_index_parser_type_from_string(config.analyzer_name);
+}
+
+TextAnalyzerPtr builtin_text_analyzer(const InvertedIndexAnalyzerConfig& 
config) {
+    return std::make_shared<LuceneBackedTextAnalyzer>(
+            
InvertedIndexAnalyzer::create_builtin_analyzer(builtin_parser_type(config),
+                                                           config.parser_mode, 
config.lower_case,
+                                                           config.stop_words));
+}
+
 class BuiltinAnalyzerProvider final : public AnalyzerProvider {
 public:
-    explicit BuiltinAnalyzerProvider(InvertedIndexAnalyzerConfig config)
-            : _analyzer(InvertedIndexAnalyzer::create_builtin_analyzer(
-                      config.analyzer_name.empty()
-                              ? config.parser_type
-                              : 
get_inverted_index_parser_type_from_string(config.analyzer_name),
-                      config.parser_mode, config.lower_case, 
config.stop_words)) {}
+    explicit BuiltinAnalyzerProvider(const InvertedIndexAnalyzerConfig& config)
+            : _analyzer(builtin_text_analyzer(config)) {}
+
+    TextAnalyzerPtr get_analyzer() const override { return _analyzer; }
+
+private:
+    const TextAnalyzerPtr _analyzer;
+};
+
+// The index's char filter in front of an analyzer. The writers and the query 
side used to apply it
+// through the reader they gave the analyzer (create_reader); it now belongs 
to the analyzer, so
+// every consumer reads every value through it, whatever the analyzer: a 
normalizer index with
+// char_filter_pattern "-" writes "A-B" as "a b". It runs the CLucene char 
filter over the whole
+// value until the char filters are native.
+class IndexCharFilterStream final : public TextTokenStream {
+public:
+    IndexCharFilterStream(const CharFilterMap* char_filters, 
std::unique_ptr<TextTokenStream> in)
+            : _char_filters(char_filters), _in(std::move(in)) {}
+
+    void reset(std::string_view text) override {
+        _filtered = InvertedIndexAnalyzer::filter_characters(*_char_filters, 
text);
+        _in->reset(_filtered);

Review Comment:
   [P2] Release oversized filtered-string storage when resetting the reusable 
stream. After a large value, assigning a short filtered result can reuse 
_filtered's existing allocation, so reserved_bytes() keeps charging the large 
buffer for every following row until segment finish. TextTokenStream::reset() 
promises high-water scratch release; call the scratch release helper on 
_filtered before replacing it.



##########
be/src/storage/index/inverted/snii/snii_index_writer.cpp:
##########
@@ -592,10 +521,11 @@ Status SniiIndexColumnWriter::finish() {
                                                      std::move(_null_docids)),
             _term_buffer.get(), _config, std::move(options), 
_memory_reporter.get());
     _report_encoded_norms_capacity(/*release_all=*/true);
-    // The tokenizer's buffers outlive this call (they die with _analyzer), 
but the reporter is
-    // about to be handed to the index file writer and become a long-lived 
process-wide counter,
-    // so its accumulation-phase charges must balance to zero first.
-    _report_gram_buffers_capacity(/*release_all=*/true);
+    // The reporter is about to be handed to the index file writer and become 
a long-lived
+    // process-wide counter, so its accumulation-phase charges must balance to 
zero first, and the
+    // stream whose buffers are no longer charged goes.
+    _report_analysis_capacity(/*release_all=*/true);
+    _release_analysis();

Review Comment:
   [P2] Release the persistent TokenBatch during analyzer-failure cleanup. If 
next_batch() throws after emitting tokens, _latch_analysis_failure() calls 
close_on_error(), which zeroes _analysis_charged_bytes and resets _stream but 
leaves _batch's arena/token capacity alive. IndexBuilder retains the failed 
writer in _index_column_writers until the build unwinds, so a large partial 
batch remains resident and invisible to the writer memory reporter. Clear and 
release the batch (or destroy it as part of _release_analysis) before dropping 
the charge.



-- 
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]

Reply via email to