meta: support storing placement policy into meta #27251
Conversation
Signed-off-by: ailinkid <[email protected]>
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
release-note
Signed-off-by: ailinkid <[email protected]>
meta/meta.go
Outdated
| @@ -144,6 +153,21 @@ func (m *Meta) GetGlobalID() (int64, error) { | |||
| return m.txn.GetInt64(mNextGlobalIDKey) | |||
| } | |||
|
|
|||
| func (m *Meta) GenPolicyID() (int64, error) { | |||
There was a problem hiding this comment.
Maybe generate the id when creating the new policy? a.k.a auto incremental id.
There was a problem hiding this comment.
@AilinKid Actually, I mean "remove this API". GenPolicyID is used outside CreatePolicy, the behavior is not controlled by meta. One could just generate a random number other than numbers from GenPolicyID. Is there any advantage with a standalone ID generating method? Otherwise I prefer less/simple/opinionated API.
Not a necessary change request. rest code LGTM
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
AilinKid
force-pushed
the
support-store-policy-in-meta
branch
from
9197a87 to
3896af7
Compare
Signed-off-by: ailinkid <[email protected]>
|
/build |
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
|
/run-check_dev_2 |
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
Signed-off-by: ailinkid <[email protected]>
|
|
||
| func detachMagicByte(value []byte) ([]byte, error) { | ||
| magic, data := value[:1], value[1:] | ||
| switch whichMagicType(magic[0]) { |
There was a problem hiding this comment.
Can you explain how should we use the magic byte in the future? Does it means if the byte <= 0x3F they must be compatible each other? For example 0x3F can read the data with byte 0x0 and 0x0 can read the data with byte 0x3F too. If we want to make an incompatible version, the byte must be upgraded to > 0x3F? And it can still be in json format, but ... incompatible.
There was a problem hiding this comment.
This point comes from wanghe. (like magic byte in Java class file)
Appending fields is naturally compatible with old version, so we don't have to change magic byte. For extreme case, modifying a old field will cause misunderstanding between versions, in which we should change the magic byte + 0x01, that told new / old version to handling in a new way / just throw a error, but it should be still in the json handler-range.
In other extreme case, one day we may use msgpack/yaml/other-serialization-format instead, where we should change the magic byte to other reserved corresponding range.
There was a problem hiding this comment.
I see, but for the code:
case typeJSON:
return data, nil
It does not make any error only if the magic number < 0x3F. Does it mean new versions must be compatible with old version if their magic numbers are in the same range.
Signed-off-by: ailinkid <[email protected]>
Eventually, I will put it into the parser, now there is import circle here, that's also why I put it in the util package |
Signed-off-by: ailinkid <[email protected]>
ti-chi-bot
added
status/LGT2
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: d318359
|
ti-chi-bot
added
the
status/can-merge
|
/run-unit-test |
Signed-off-by: ailinkid [email protected]
What problem does this PR solve?
Issue Number: close #27250
Support store the placement policy into meta
What is changed and how it works?
What's Changed:
support storing placement policy into meta. It's has a global scope and is at the same level like schema(database) but it doesn't share the namespace and idspace with any other elements.
Check List
Tests
Release note