Copilot commented on code in PR #13826:
URL: https://github.com/apache/apisix/pull/13826#discussion_r3782280668
##########
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:
该表格包含“有效值”列，但新增的 max_pending_entries 行该列为空。由于 schema 最小值为 1，这里建议补充 ">= 1"
以避免歧义。
##########
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:
When the backlog limit is exceeded, add_entry() returns nil. Callers
typically interpret any falsy return as “no existing processor” and then call
add_entry_to_new_processor(), which triggers should_discard() a second time and
can double-count/log discarded_entries for a single dropped entry.
This issue also appears on line 177 of the same file.
##########
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:
This plugin now exposes max_pending_entries via metadata_schema, but the
batch processor manager is instantiated earlier as bp_manager_mod.new("lago
logger") (no plugin_name argument). That means batch-processor-manager will
look up metadata under "lago logger" rather than the plugin name "lago", so
max_pending_entries overrides in plugin metadata won’t take effect.
##########
docs/en/latest/plugins/loggly.md:
##########
@@ -74,6 +74,7 @@ You can also configure the Plugin through Plugin metadata.
The following configu
| protocol | string | False | "syslog" | [ "syslog" ,
"http", "https" ] | Protocol in which the logs are sent to Loggly.
|
| log_format | object | False | nil |
| Log format declared as key-value pairs in JSON. Values support
strings and nested objects (up to five levels deep; deeper fields are
truncated). Within strings, [APISIX](../apisix-variable.md) or
[NGINX](http://nginx.org/en/docs/varindex.html) variables can be referenced by
prefixing with `$`. |
| log_format_extra | object | False | |
| Extra log fields **added on top of** the default log entry,
keeping every default field instead of replacing them (unlike `log_format`).
Same value syntax as `log_format`. Ignored when `log_format` is set. |
+| max_pending_entries | integer | False | 16384 | | Maximum number of entries
waiting to be processed. New entries are discarded while the backlog exceeds
this, which stops a slow or unreachable log server from growing the worker's
memory without bound. See [Batch
Processor](../batch-processor.md#limiting-the-backlog) for the memory a backlog
of this size costs. |
Review Comment:
The metadata table includes a “Valid values” column, but the new
max_pending_entries row leaves it blank. Since the schema enforces minimum 1,
documenting ">= 1" here would align with other plugin docs and avoid ambiguity.
##########
docs/zh/latest/plugins/loggly.md:
##########
@@ -68,6 +68,7 @@ description: API 网关 Apache APISIX loggly 插件可用于将日志转发到 S
| timeout | integer | 否 | 5000 |
| 发送数据请求超时时间（以毫秒为单位）。 |
| protocol | string | 否 | "syslog" | [ "syslog", "http",
"https" ] | 将日志发送到 Loggly 的协议。 |
| log_format | object | 否 | nil |
| 日志格式以 JSON 的键值对声明。值支持字符串和嵌套对象（最多五层，超出部分将被截断）。字符串中可通过在前面加上 `$` 来引用
[APISIX 变量](../../../en/latest/apisix-variable.md) 或 [NGINX
内置变量](http://nginx.org/en/docs/varindex.html)。 |
+| max_pending_entries | integer | 否 | 16384 | |
待处理条目数的上限。积压超过该值后新条目会被丢弃，避免日志服务变慢或不可达时 worker 内存无限增长。该上限对应的内存开销参见
[批处理器](../batch-processor.md#限制积压条目数)。 |
Review Comment:
该表格包含“有效值”列，但新增的 max_pending_entries 行该列为空。由于 schema 最小值为 1，这里建议补充 ">= 1"
以避免歧义，并与其他插件文档保持一致。
--
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]
