feat(clustering/rpc): change sync version to string #14078
Conversation
github-actions
bot
added
core/clustering
core/db
cherry-pick kong-ee
| function _M:init_cp(manager) | ||
| local purge_delay = manager.conf.cluster_data_plane_purge_delay | ||
|
|
||
| -- CP | ||
| -- Method: kong.sync.v2.get_delta | ||
| -- Params: versions: list of current versions of the database | ||
| -- example: { default = { version = 1000, }, } | ||
| -- example: { default = { version = "1000", }, } |
There was a problem hiding this comment.
-- -- example: { default = { version = "1000", }, }
++ -- example: { default = { version = "v1000", }, }miss a V?
There was a problem hiding this comment.
There are many places of such examples, but I think that they are just an explanation, need not be exact code.
There was a problem hiding this comment.
If we do think that we should update them, we could do it in another PR.
| local byte = string.byte | ||
| local CHAR_V = byte("V") | ||
|
|
||
| -- version string must start with char 'V' |
There was a problem hiding this comment.
I feel this format is not good for future extensibility. Maybe:
02xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Where xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx is the hex encoded version number. Essentially: "02%030x". This allows us to somewhat distinguish in the future when a new format is used. We can simply add 03, 04, etc.
There was a problem hiding this comment.
If the string is like 02xxx then we can not distinguish between a hex string (ngx.md5) and a version string.
e.g.: a v1 config hash starts with 02 (02ABCDEF...) by chance.
There was a problem hiding this comment.
Perhaps we could set the version string a V0xxx V1xxx
There was a problem hiding this comment.
@chronolaw The version string is provided by the DP, and DP should know if it is a valid version or not. If the string is not valid, then DP should simply send 02000000000000000000000000000000 instead. Does CP need to know about this detail?
There was a problem hiding this comment.
As I said above, if the version string stored in lmdb is 02ABCDEF..., and it is a config hash generated by a YAML by change, how DP could judge it valid?
|
|
||
| local is_valid_version | ||
| do | ||
| local VER_PREFIX = "V02_" |
There was a problem hiding this comment.
| local VER_PREFIX = "V02_" | |
| local VER_PREFIX = "v02_" |
I still prefer lowercased v, this looks like the format others use (e.g. GitHub tokens).
Summary
KAG-5894
Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
Fix #[issue number]