Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
github-actions[bot] commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-4459132853 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-4037589312 Hi @xuruidong, following up on the previous review comments. Please let us know if you have any updates. Thank you. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Copilot commented on code in PR #12011:
URL: https://github.com/apache/apisix/pull/12011#discussion_r2844094837
##
apisix/core/config_etcd.lua:
##
@@ -807,22 +808,23 @@ local function sync_data(self)
-- avoid space waste
if self.sync_times > 100 then
-local values_original = table.clone(self.values)
-table.clear(self.values)
-
-for i = 1, #values_original do
-local val = values_original[i]
+local pre = 1
+local cur = 1
+table.clear(self.values_hash)
+log.info("clear stale data in `values_hash` for key: ", key)
+for _, val in ipairs(self.values) do
if val then
-table.insert(self.values, val)
+self.values[pre] = val
+key = short_key(self, val.key)
+self.values_hash[key] = pre
+pre = pre + 1
end
-end
-table.clear(self.values_hash)
-log.info("clear stale data in `values_hash` for key: ", key)
+cur = cur + 1
+end
-for i = 1, #self.values do
-key = short_key(self, self.values[i].key)
-self.values_hash[key] = i
+for i = cur - 1, pre, -1 do
Review Comment:
The `cur` variable is only used to track the iteration count and could be
replaced with `#self.values` captured before the loop. For example: `local
original_len = #self.values` before line 815, then use `original_len` instead
of `cur - 1` in line 826. This would eliminate the need for incrementing `cur`
on every iteration and make the code slightly clearer.
##
apisix/core/config_etcd.lua:
##
@@ -807,22 +808,23 @@ local function sync_data(self)
-- avoid space waste
if self.sync_times > 100 then
-local values_original = table.clone(self.values)
-table.clear(self.values)
-
-for i = 1, #values_original do
-local val = values_original[i]
+local pre = 1
+local cur = 1
+table.clear(self.values_hash)
+log.info("clear stale data in `values_hash` for key: ", key)
+for _, val in ipairs(self.values) do
if val then
-table.insert(self.values, val)
+self.values[pre] = val
+key = short_key(self, val.key)
+self.values_hash[key] = pre
+pre = pre + 1
end
-end
-table.clear(self.values_hash)
-log.info("clear stale data in `values_hash` for key: ", key)
+cur = cur + 1
+end
-for i = 1, #self.values do
-key = short_key(self, self.values[i].key)
-self.values_hash[key] = i
+for i = cur - 1, pre, -1 do
+remove_tab(self.values, i)
end
Review Comment:
The refactored array compaction logic (lines 811-828) lacks test coverage.
This is a critical code path that triggers after 100 deletions to clean up the
values array. Consider adding a test that creates multiple items, deletes them
to increment sync_times beyond 100, and verifies the array is correctly
compacted with no data corruption.
##
apisix/core/config_etcd.lua:
##
@@ -807,22 +808,23 @@ local function sync_data(self)
-- avoid space waste
if self.sync_times > 100 then
-local values_original = table.clone(self.values)
-table.clear(self.values)
-
-for i = 1, #values_original do
-local val = values_original[i]
+local pre = 1
+local cur = 1
Review Comment:
The variable names `pre` and `cur` are unclear and could be more
descriptive. Consider renaming to `write_idx` and `read_idx` respectively to
better convey their purpose in the compaction algorithm. `write_idx` represents
the next position to write a valid element, while `read_idx` tracks the current
position being read from the original array.
##
apisix/core/config_etcd.lua:
##
@@ -807,22 +808,23 @@ local function sync_data(self)
-- avoid space waste
if self.sync_times > 100 then
-local values_original = table.clone(self.values)
-table.clear(self.values)
-
-for i = 1, #values_original do
-local val = values_original[i]
+local pre = 1
+local cur = 1
+table.clear(self.values_hash)
+log.info("clear stale data in `values_hash` for key: ", key)
+for _, val in ipairs(self.values) do
if val then
-table.insert(self.values, val)
+self.values[pre] = val
+key = short_key(self, val.key)
+
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
github-actions[bot] commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3940630932 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
github-actions[bot] commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3631386016 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3344640477 Hi @xuruidong, are you still working on this pull request? There are some failed tests that need to be addressed. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3389120654 > Hi @xuruidong, are you still working on this pull request? There are some failed tests that need to be addressed. Hi @Baoyuantop ，please trigger the CI -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
github-actions[bot] commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3341468290 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3130357140 > Hi @xuruidong, any updates? Hi @Baoyuantop ,please trigger the ci -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3125894209 Hi @xuruidong, any updates? -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-3096008047 Hi @xuruidong, there are still bad CIs that need to be fixed. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2986481930 Hi @Baoyuantop , the ci is not stable, the failed test should be rerun -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2978797335 > Please fix the failed ci Hi @Baoyuantop ,please trigger the ci -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2975503155 Please fix the failed ci -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2868859153 > Hi @xuruidong, please add a PR description so that others can review it better ~ Hi @Baoyuantop , the PR description has been added, please help review it. Thanks. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2835532243 Hi @xuruidong, please add a PR description so that others can review it better ~ -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
Baoyuantop commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2753598502 Hi @xuruidong, can you add some descriptive information for PR? -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2727918243 I don't think the CI failure is related to the code changes. -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
juzhiyuan commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2709344552 > > @xuruidong Just approved to run the CI > > please run the CI again @juzhiyuan Hi @xuruidong, I couldn't find the Re-run button. Can you confirm the failed test cases? https://github.com/user-attachments/assets/5bde37fd-501f-426c-8071-99b809d05635"; /> -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
xuruidong commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2706139891 > @xuruidong Just approved to run the CI please run the CI again @juzhiyuan -- 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]
Re: [PR] perf(etcd): refactor some code to improve performance [apisix]
juzhiyuan commented on PR #12011: URL: https://github.com/apache/apisix/pull/12011#issuecomment-2696317750 @xuruidong Just approved to run the CI -- 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]
