nic-6443 commented on code in PR #13826:
URL: https://github.com/apache/apisix/pull/13826#discussion_r3782302472
##########
apisix/utils/batch-processor-manager.lua:
##########
@@ -98,16 +129,37 @@ local function total_processed_entries(self)
return processed_entries
end
-function _M:add_entry(conf, entry, max_pending_entries)
- if max_pending_entries then
- local total_processed_entries_count = total_processed_entries(self)
- if self.total_pushed_entries - total_processed_entries_count >
max_pending_entries then
- core.log.error("max pending entries limit exceeded. discarding
entry.",
- " total_pushed_entries: ",
self.total_pushed_entries,
- " total_processed_entries: ",
total_processed_entries_count,
- " max_pending_entries: ", max_pending_entries)
- return
- end
+
+local function should_discard(self)
+ local metadata = plugin.plugin_metadata(self.plugin_name)
+ local max_pending_entries = metadata and metadata.value
+ and metadata.value.max_pending_entries
+ or DEFAULT_MAX_PENDING_ENTRIES
+
+ local total_processed_entries_count = total_processed_entries(self)
+ if self.total_pushed_entries - total_processed_entries_count <=
max_pending_entries then
+ return false
+ end
+
+ self.discarded_entries = self.discarded_entries + 1
+ local time = now()
+ if time - self.last_discard_log_time >= DISCARD_LOG_INTERVAL then
+ core.log.error("max pending entries limit exceeded. discarding entry.",
+ " total_pushed_entries: ", self.total_pushed_entries,
+ " total_processed_entries: ",
total_processed_entries_count,
+ " max_pending_entries: ", max_pending_entries,
+ " discarded_entries: ", self.discarded_entries)
+ self.last_discard_log_time = time
+ self.discarded_entries = 0
+ end
+
+ return true
+end
+
+
+function _M:add_entry(conf, entry)
+ if should_discard(self) then
+ return
end
Review Comment:
Fixed in the follow-up commit. It was worse than a wasted check: `add_entry`
counted the discard, the caller fell through, and `add_entry_to_new_processor`
counted the same entry again, so the reported `discarded_entries` was about
double the real loss.
Counting and reporting now happen only in `add_entry`.
`add_entry_to_new_processor` still checks the backlog, so a caller that reaches
it directly stays bounded, but it no longer counts. The test asserts the exact
reported counts (`1`, then `99` after the interval), which fails on the doubled
count.
##########
docs/zh/latest/plugins/sls-logger.md:
##########
@@ -85,6 +85,7 @@ title: sls-logger
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述
|
| ---------------- | ------- | ------ | ------------- | ------- |
------------------------------------------------ |
| log_format | object | 可选 | | | 日志格式以 JSON
的键值对声明。值支持字符串和嵌套对象（最多五层，超出部分将被截断）。字符串中可通过在前面加上 `$` 来引用 [APISIX
变量](../../../en/latest/apisix-variable.md) 或 [Nginx
内置变量](http://nginx.org/en/docs/varindex.html)。特别的，**该设置是全局生效的**，意味着指定
log_format 后，将对所有绑定 sls-logger 的 Route 或 Service 生效。 |
+| max_pending_entries | integer | 可选 | 16384 | |
待处理条目数的上限。积压超过该值后新条目会被丢弃，避免日志服务变慢或不可达时 worker 内存无限增长。该上限对应的内存开销参见
[批处理器](../batch-processor.md#限制积压条目数)。 |
Review Comment:
Fixed — `>= 1` added here and in the two `loggly` tables that had the same
gap.
##########
apisix/plugins/lago.lua:
##########
@@ -120,15 +120,26 @@ schema = batch_processor_manager:wrap_schema(schema)
schema.properties.batch_max_size.default = 100
+local metadata_schema = batch_processor_manager:wrap_metadata_schema({
+ type = "object",
+ properties = {},
+})
Review Comment:
Correct, and a bug I introduced: the script that rewrote the plugins handled
lago separately and I missed this line, so it looked its metadata up under
`lago logger` and silently ignored any override. Fixed.
I also added a test for the whole class rather than just this one plugin: it
reloads every logger module with `bp_manager_mod.new` wrapped and asserts each
manager's `plugin_name` is a real logger plugin. Reverting only this line makes
it fail.
--
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]
