feat[manual-only]: implement retrival of licenses from CLS server#35246
Open
acerDebugman wants to merge 10 commits intomainfrom
Open
feat[manual-only]: implement retrival of licenses from CLS server#35246acerDebugman wants to merge 10 commits intomainfrom
acerDebugman wants to merge 10 commits intomainfrom
Conversation
Define CLS response grant storage types and limits for SDB. Support cached CLS grant state in mnode metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Clamp altered CLS refresh intervals and preserve the last explicit value. Use derived refresh intervals during clsEnabled state transitions and cover them with tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move ALTER normalization to mnd config handling and drop the extra tconfig clamp path. Narrow the cached refresh interval type to int32. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename the persisted CLS grant type and SDB key to the grant cls naming. Add extend payload fields to the shared metadata struct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename the persisted CLS grant enum to match the grant cache naming. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements the CLS (Cloud License Service) configuration and heartbeat functionality, adding new global variables, message types, and dynamic configuration logic. The review feedback identifies a potential performance issue where the message queue could be flooded when CLS is disabled, and highlights several code quality improvements, including the removal of dead code and redundant TODO comments. Additionally, the reviewer noted inconsistencies in interval validation ranges, the use of magic numbers, and suggested returning errors for invalid inputs instead of silently clamping values.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CLS (license retrieval) configuration and scheduling hooks so enterprise builds can periodically pull license data from a CLS server, including config validation/normalization and new message/SDB type definitions.
Changes:
- Introduce CLS-related global configs (
clsEnabled,clsUrl,clsLicenseId,clsRefreshInterval) and dynamic update handling. - Add an mnode timer pullup path + new RPC timer message type (
TDMT_MND_CLS_HB_TIMER) for CLS refresh. - Add/adjust unit tests intended to cover CLS dynamic config behavior.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| source/util/test/cfgTest.cpp | Adds a unit test for clsRefreshInterval alter/range behavior. |
| source/util/src/tconfig.c | Minor formatting/line alignment change. |
| source/dnode/mnode/sdb/inc/sdb.h | Adds new SDB type SDB_GRANT_CLS and bumps SDB_MAX. |
| source/dnode/mnode/impl/src/mndMain.c | Adds CLS timer pullup and includes CLS timer msg in state checks. |
| source/dnode/mnode/impl/src/mndConfig.c | Normalizes clsRefreshInterval on alter requests (enterprise-only). |
| source/dnode/mnode/impl/inc/mndDef.h | Adds SGrantClsObj structure for CLS grant/response storage. |
| source/common/test/commonTests.cpp | Adds a fixture/test for CLS dynamic config transitions. |
| source/common/src/tglobal.c | Adds CLS globals + validation and enable/disable transition handling. |
| include/util/tdef.h | Adds TSDB_GRANT_CLS_RESP_LEN constant. |
| include/common/tmsgdef.h | Adds new message type TDMT_MND_CLS_HB_TIMER. |
| include/common/tglobal.h | Exposes CLS globals via extern declarations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Set the public cls refresh interval range to 10..86400 and simplify the derived interval helpers. Drop stale commented code in the timer pullup path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Enterprise-only configuration knobs and timer plumbing intended to periodically refresh CLS (license) information from a CLS server.
Changes:
- Introduces new global config options for CLS (
clsEnabled,clsUrl,clsLicenseId,clsRefreshInterval) and dynamic-update handling. - Adds a new mnode timer message type (
TDMT_MND_CLS_HB_TIMER) and schedules periodic CLS pullups. - Adds/updates unit tests around CLS dynamic config behavior and config range handling.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| source/util/test/cfgTest.cpp | Adds a new unit test for clsRefreshInterval alter/range behavior. |
| source/util/src/tconfig.c | No functional change (formatting/line alignment). |
| source/dnode/mnode/sdb/inc/sdb.h | Adds SDB_GRANT_CLS enum entry and bumps SDB_MAX. |
| source/dnode/mnode/impl/src/mndMain.c | Adds CLS timer pullup builder/scheduler and allows timer bypass during restore/stop checks. |
| source/dnode/mnode/impl/src/mndConfig.c | Adds normalization/clamping for clsRefreshInterval on alter requests. |
| source/dnode/mnode/impl/inc/mndDef.h | Adds SGrantClsObj state struct. |
| source/common/test/commonTests.cpp | Adds Enterprise-only test for CLS enable/refresh interval transition behavior. |
| source/common/src/tglobal.c | Defines new CLS globals, adds config registration, validation, and dynamic update logic. |
| include/util/tdef.h | Adds TSDB_GRANT_CLS_RESP_LEN size constant. |
| include/common/tmsgdef.h | Adds new message type definition TDMT_MND_CLS_HB_TIMER. |
| include/common/tglobal.h | Exposes CLS globals via extern declarations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Match cfg-layer behavior by rejecting out-of-range updates\nand keeping the configured value unchanged when set fails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes 6665336277
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.