enable & disable checkpoint

[jiangxinmeng1]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #17034

What this PR does / why we need it:

enable & disable checkpoint

---

PR Type

Enhancement

---

Description

• Added OpDisableCheckpoint to OpCode enum and updated related maps.
• Introduced openTime field to runner struct and added logic to disable checkpointing if the runner has been running for less than 20 minutes.
• Added handling for OpDisableCheckpoint in Debug function.
• Added DisableCKPMethod constant and handler.
• Implemented HandleDisableCheckpoint function to handle enabling and disabling checkpoints.
• Added HandleDisableCheckpoint method to Handler interface.
• Added log messages to DisableCheckpoint and EnableCheckpoint methods.
• Added Enable field to Checkpoint struct and message.

---

Changes walkthrough 📝

Relevant files

Enhancement

10 files api.pb.go Add `OpDisableCheckpoint` to `OpCode` enum and related maps pkg/pb/api/api.pb.go Added OpDisableCheckpoint to OpCode enum. Updated OpCode_name and OpCode_value maps to include OpDisableCheckpoint. +177/-174 runner.go Add openTime to runner and logic to disable checkpointing based on runtime pkg/vm/engine/tae/db/checkpoint/runner.go Added openTime field to runner struct. Initialized openTime in NewRunner function. Added logic to disable checkpointing if the runner has been running for less than 20 minutes. +8/-0      storage_debug.go Handle `OpDisableCheckpoint` in Debug function                      pkg/txn/storage/tae/storage_debug.go Added case for OpDisableCheckpoint in Debug function to handle disabling checkpoints. +12/-1    types.go Add `DisableCKPMethod` constant and handler                            pkg/sql/plan/function/ctl/types.go Added DisableCKPMethod constant. Added DisableCKPMethod to the method handler map. +2/-0      handle_debug.go Add `HandleDisableCheckpoint` function                                      pkg/vm/engine/tae/rpc/handle_debug.go Added HandleDisableCheckpoint function to handle enabling and disabling checkpoints. +16/-0    handler.go Add `HandleDisableCheckpoint` method to Handler interface pkg/vm/engine/tae/iface/rpchandle/handler.go Added HandleDisableCheckpoint method to Handler interface. +6/-0      testutils.go Add log messages to checkpoint enable/disable methods        pkg/vm/engine/tae/db/checkpoint/testutils.go Added log messages to DisableCheckpoint and EnableCheckpoint methods. +2/-0      operations.go Add `Enable` field to Checkpoint struct                                    pkg/vm/engine/tae/db/operations.go Added Enable field to Checkpoint struct. +1/-0      api.proto Add `OpDisableCheckpoint` to OpCode enum                                  proto/api.proto Added OpDisableCheckpoint to OpCode enum. +12/-11  operations.proto Add `Enable` field to Checkpoint message                                  pkg/vm/engine/tae/db/operations.proto Added Enable field to Checkpoint message. +1/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	3
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The log message "Checkpoint is disable" in the runner.go file might be a typo and should be "Checkpoint is disabled".
	Consistency Issue: The logging for enabling and disabling checkpoints in testutils.go uses "Checkpoint is Disable" and "Checkpoint is Enable", which should be corrected to "Checkpoint is Disabled" and "Checkpoint is Enabled" for grammatical consistency.
	Design Consideration: The implementation of the checkpoint disable feature directly checks the system time since the runner was opened to determine if checkpointing should be disabled. This might not be flexible for different operational requirements or testing scenarios. Consider adding a configuration option or method to set the time threshold dynamically.

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Simplify the method by using a single return statement with a nil check The GetEnable method can be simplified by using a single return statement with a nil check. This reduces the number of lines and makes the code more concise. pkg/vm/engine/tae/db/operations.pb.go [178-181] -if m != nil { - return m.Enable -} -return false +return m != nil && m.Enable Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies an opportunity to simplify the GetEnable method, improving readability and conciseness without altering functionality.	8
	Correct the grammar in the log message for better readability The log message "Checkpoint is disable" should be corrected to "Checkpoint is disabled" for proper grammar. pkg/vm/engine/tae/db/checkpoint/runner.go [768] -logutil.Infof("Checkpoint is disable") +logutil.Infof("Checkpoint is disabled") Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion correctly identifies a grammatical error in the log message, improving readability.	6
	Use iota to automatically increment constant values for conciseness and error reduction Consider using iota to automatically increment the OpCode values, which can reduce the risk of manual errors and make the code more concise. pkg/pb/api/api.pb.go [31-47] const ( - OpCode_Nop OpCode = 0 - OpCode_OpGetLogTail OpCode = 1000 - OpCode_OpPreCommit OpCode = 1001 - OpCode_OpPing OpCode = 2000 - OpCode_OpFlush OpCode = 2001 - OpCode_OpCheckpoint OpCode = 2003 - OpCode_OpInspect OpCode = 2004 - OpCode_OpAddFaultPoint OpCode = 2005 - OpCode_OpBackup OpCode = 2006 - OpCode_OpTraceSpan OpCode = 2007 - OpCode_OpStorageUsage OpCode = 2008 - OpCode_OpGlobalCheckpoint OpCode = 2009 - OpCode_OpInterceptCommit OpCode = 2010 - OpCode_OpCommitMerge OpCode = 2011 - OpCode_OpDisableCheckpoint OpCode = 2012 + OpCode_Nop OpCode = iota + _ + _ + OpCode_OpGetLogTail = 1000 + iota - 3 + OpCode_OpPreCommit + _ + _ + OpCode_OpPing = 2000 + iota - 5 + OpCode_OpFlush + _ + OpCode_OpCheckpoint + OpCode_OpInspect + OpCode_OpAddFaultPoint + OpCode_OpBackup + OpCode_OpTraceSpan + OpCode_OpStorageUsage + OpCode_OpGlobalCheckpoint + OpCode_OpInterceptCommit + OpCode_OpCommitMerge + OpCode_OpDisableCheckpoint ) Apply this suggestion Suggestion importance[1-10]: 5 Why: Using iota could simplify the definition of constants, but the suggested implementation might not be directly applicable due to non-sequential and grouped values in the existing constants.	5
Best practice	Add a test case for the new constant to ensure proper handling Add a test case to ensure that the new OpCode_OpDisableCheckpoint value is correctly handled in all relevant parts of the codebase, such as serialization and deserialization. pkg/pb/api/api.pb.go [46] -OpCode_OpDisableCheckpoint OpCode = 2012 +OpCode_OpDisableCheckpoint OpCode = 2012 // Add test case for this value Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding a test case for the new OpCode_OpDisableCheckpoint is crucial to ensure it is handled correctly across the codebase, impacting reliability and correctness.	8
	Add unit tests to verify the behavior of the new method Consider adding a unit test for the GetEnable method to ensure it behaves correctly when m is nil and when m.Enable is true or false. pkg/vm/engine/tae/db/operations.pb.go [177-182] -func (m *Checkpoint) GetEnable() bool { - if m != nil { - return m.Enable +// Unit test for GetEnable method +func TestGetEnable(t *testing.T) { + var m *Checkpoint + if m.GetEnable() != false { + t.Error("Expected false when m is nil") } - return false + + m = &Checkpoint{Enable: true} + if m.GetEnable() != true { + t.Error("Expected true when m.Enable is true") + } + + m = &Checkpoint{Enable: false} + if m.GetEnable() != false { + t.Error("Expected false when m.Enable is false") + } } Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding unit tests is a best practice that ensures the new method works as expected under various conditions. This suggestion is relevant and promotes robust code.	7
	Rename the method to follow Go naming conventions for boolean getters Consider renaming the GetEnable method to IsEnabled to follow Go naming conventions for boolean getters, making the code more idiomatic. pkg/vm/engine/tae/db/operations.pb.go [177-182] -func (m *Checkpoint) GetEnable() bool { +func (m *Checkpoint) IsEnabled() bool { if m != nil { return m.Enable } return false } Apply this suggestion Suggestion importance[1-10]: 7 Why: Renaming GetEnable to IsEnabled aligns with Go naming conventions for boolean methods, enhancing code readability and maintainability.	7
Maintainability	Add comments to group related constants for better readability Consider adding a comment above the OpCode constants to explain the purpose of each group of constants (e.g., general operations, logging operations, etc.). This will improve code readability and maintainability. pkg/pb/api/api.pb.go [31-47] +// General operations const ( OpCode_Nop OpCode = 0 OpCode_OpGetLogTail OpCode = 1000 OpCode_OpPreCommit OpCode = 1001 +) +// Logging operations +const ( OpCode_OpPing OpCode = 2000 OpCode_OpFlush OpCode = 2001 OpCode_OpCheckpoint OpCode = 2003 OpCode_OpInspect OpCode = 2004 OpCode_OpAddFaultPoint OpCode = 2005 OpCode_OpBackup OpCode = 2006 OpCode_OpTraceSpan OpCode = 2007 OpCode_OpStorageUsage OpCode = 2008 OpCode_OpGlobalCheckpoint OpCode = 2009 OpCode_OpInterceptCommit OpCode = 2010 OpCode_OpCommitMerge OpCode = 2011 OpCode_OpDisableCheckpoint OpCode = 2012 ) Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding comments to group related constants would indeed improve readability and maintainability, especially given the large number of constants.	7
Possible issue	Verify the uniqueness of new constant values to prevent conflicts Ensure that the new OpCode values do not conflict with any existing values in other parts of the codebase. This can prevent potential runtime errors due to duplicate values. pkg/pb/api/api.pb.go [46] -OpCode_OpDisableCheckpoint OpCode = 2012 +OpCode_OpDisableCheckpoint OpCode = 2012 // Ensure this value is unique Apply this suggestion Suggestion importance[1-10]: 6 Why: Ensuring the uniqueness of new constant values is important to prevent runtime errors, but the suggestion lacks specific implementation details or checks.	6
	Ensure the Enable field is properly initialized to avoid unexpected behavior Ensure that the Enable field is properly initialized in all instances of the Checkpoint struct to avoid unexpected behavior. pkg/vm/engine/tae/db/operations.pb.go [177-182] -func (m *Checkpoint) GetEnable() bool { - if m != nil { - return m.Enable - } - return false +// Ensure Enable field is initialized +func NewCheckpoint(enable bool) *Checkpoint { + return &Checkpoint{Enable: enable} } Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion to ensure initialization of the Enable field is valid to prevent potential bugs, though it assumes that Enable is not already initialized elsewhere in the codebase.	6