refactor(clustering): small improvements about incremental sync

[chronolaw]

Summary

KAG-5768
KAG-5776
KAG-5780
KAG-5788

Checklist

• The Pull Request has tests
• A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

[StarlightIbuki]

I think we still need the log for debugging. It will be hard to diagnose if we log nothing when succeeds

[chronolaw]

See comment: https://github.com/Kong/kong-ee/pull/10460#discussion_r1831761668

[chronolaw]

Let's remove it now, If we do need it in the practice we could add them back.

[StarlightIbuki]

@chronolaw With the level debug it's almost free in runtime. Any specific reason we are removing them?

[bungle]

@StarlightIbuki we need to be careful with debug logs. Currently we don't have means of disabling just some debug logs or fine tune what is debug logged. While log messages like these may be beneficial when debugging incremental sync, it makes debugging everything else much harder as logs are just flooded with these. Logs that happen by background workers and timers or automated processes are especially problematic. And we already log errors. Unix: no news is good news.

[bungle]

Question.

[bungle]

Can we initialize this once and then just reuse, e.g. a module level get_latest_version:

Suggested change

	local strategy = require("kong.clustering.services.sync.strategies.postgres").new(db)
	if not get_latest_version then
	local strategy = require("kong.clustering.services.sync.strategies.postgres").new(db)
	get_latest_version = function()
	return strategy:get_latest_version()
	end

Or put it in some helper as it seems to be used in many places.

[chronolaw]

It needs the global variable kong.db, I think that it can not be a module level function.

[bungle]

module level function that caches it on first use

[chronolaw]

"kong.clustering.services.sync.strategies.postgres.new() needs a db param, if it is a module level function, kong.db may not be initialized yet.

[bungle]

@chronolaw I meant something like this:

local get_latest_version do
  local strategy
  local function get_latest_version_real()
    return strategy:get_latest_version()
  end 
  function get_latest_version()
    strategy = require("kong.clustering.services.sync.strategies.postgres").new(kong.db)
    get_latest_version = get_latest_version_real
    return get_latest_version()
  end
end

[chronolaw]

done, many thanks.

[team-gateway-bot]

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-13841-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-13841-to-master-to-upstream
git checkout -b cherry-pick-13841-to-master-to-upstream
ancref=$(git merge-base 5f5f7d5ea51ac16263b32c1446a08e00816150f9 f6580d5fd317b6323f2db5f263c2410d8263ddc6)
git cherry-pick -x $ancref..f6580d5fd317b6323f2db5f263c2410d8263ddc6