Pick print more ckp details to 1.1

[XuPeng-SH]

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #12741

What this PR does / why we need it:

print more checkpoint details

[matrix-meow]

@XuPeng-SH Thanks for your contributions!

Here are review comments for file pkg/vm/engine/tae/db/checkpoint/runner.go:
Review:

Title: Pick print more ckp details to 1.1

Body: The body of the pull request is brief and does not provide much information. It states that the PR is an improvement and mentions that it prints more checkpoint details. It also references issue #12741.

Changes Made:

• Added import statement for the "go.uber.org/zap" package.
• Modified the "onGlobalCheckpointEntries" function to remove the saving of checkpoint details and the logging of checkpoint completion.
• Modified the "onIncrementalCheckpointEntries" function to add logging statements for checkpoint start and end, and to remove the logging of checkpoint completion.
• Modified the "saveCheckpoint" function to remove the logging of checkpoint completion.
• Modified the "doIncrementalCheckpoint" function to add logging statements for checkpoint start and end, and to return additional fields for logging.
• Modified the "doGlobalCheckpoint" function to add logging statements for checkpoint start and end, and to remove the logging of checkpoint completion.

Problems:

1. Inconsistent Logging: The logging statements related to checkpoint completion have been removed from multiple functions, but the logging statements for checkpoint start and end have been added inconsistently. It would be better to have consistent logging throughout the codebase.

Suggestions:

1. Consistent Logging: Consider adding logging statements for checkpoint completion in all relevant functions to maintain consistency in the codebase. This will make it easier to track the progress and completion of checkpoints.

Optimizations:

1. Code Duplication: There is some code duplication in the "onIncrementalCheckpointEntries" and "doIncrementalCheckpoint" functions. The logging statements for checkpoint start and end can be extracted into a separate function to avoid duplication.

Suggestions:

1. Extract Logging Function: Create a separate function to handle the logging of checkpoint start and end in the "onIncrementalCheckpointEntries" and "doIncrementalCheckpoint" functions. This will reduce code duplication and make the code more maintainable.

Security Issues:
No security issues were identified in the provided pull request.

Here are review comments for file pkg/vm/engine/tae/db/checkpoint/testutils.go:
Review:

Title: Pick print more ckp details to 1.1

Body: The body of the pull request provides some context about the changes being made. It mentions that the purpose of the PR is to print more checkpoint details. However, it does not provide any specific details about what these checkpoint details are or why they are needed. It would be helpful to have more information in the body of the PR to understand the motivation behind the changes.

Changes in testutils.go:

1. Addition of import statement:

   • The pull request adds the import statement "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/wal". This import statement is necessary for the changes being made in the code.

2. Addition of log statements:

   • The pull request adds several log statements using the zap logger. These log statements are used to print information about the checkpoint process, including the start and end of the checkpoint, any errors encountered, and the time taken for the checkpoint. These log statements can be helpful for debugging and monitoring the checkpoint process.

3. Changes in ForceIncrementalCheckpoint function:

   • The pull request modifies the ForceIncrementalCheckpoint function to include additional error handling and logging. It introduces a new now variable to store the current time, which is used to calculate the time taken for the checkpoint. It also adds a defer statement to log the end of the checkpoint process, including any errors encountered and the time taken.

4. Changes in ForceCheckpointForBackup function:

   • The pull request modifies the ForceCheckpointForBackup function to include additional error handling and logging. It introduces a new now variable to store the current time, which is used to calculate the time taken for the checkpoint.

Suggestions:

1. Provide more details in the body of the pull request:

   • It would be helpful to have more information in the body of the pull request to understand the motivation behind the changes. Specifically, it would be useful to know what additional checkpoint details are being printed and why they are needed.

2. Consider adding comments to the code:

   • The changes made in the pull request involve adding log statements and error handling. It would be beneficial to add comments to the code to explain the purpose of these changes and how they improve the codebase.

3. Review error handling:

   • The pull request introduces additional error handling in the ForceIncrementalCheckpoint and ForceCheckpointForBackup functions. It would be a good idea to review this error handling to ensure that all possible error scenarios are handled correctly and that appropriate error messages are logged.

4. Optimize the code:

   • The changes made in the pull request seem to be focused on logging and error handling. While these changes can be useful for debugging and monitoring, it is important to ensure that they do not introduce any performance overhead. It would be a good idea to review the code and consider any optimizations that can be made to improve performance.

5. Consider adding unit tests:

   • The changes made in the pull request involve modifying existing functions. It would be beneficial to add unit tests to ensure that these functions work as expected and that the new logging and error handling does not introduce any regressions.

Overall, the changes made in the pull request seem to be focused on improving the logging and error handling in the checkpoint process. However, more information is needed to fully understand the motivation behind these changes. Additionally, it would be beneficial to review the error handling and consider any optimizations that can be made to improve performance.

Here are review comments for file pkg/vm/engine/tae/logtail/utils.go:
Review:

Title: Pick print more ckp details to 1.1

Body: The body of the pull request provides some context about the changes being made. It mentions that the purpose of the PR is to print more checkpoint details. However, it does not provide any specific details about what kind of details are being printed or why this change is necessary. It would be helpful to have more information in the body of the PR to understand the motivation behind the change.

Changes in utils.go:

• Line 21: Importing the "go.uber.org/zap" package.
• Line 423-474: Addition of a new function IDXString that returns a string representation of an index value.
• Line 2486-2558: Addition of a new method ExportStats that exports statistics about the checkpoint data.
• Line 2560: Addition of a new method Close that closes the checkpoint data.

Potential Problems and Suggestions:

1. Lack of documentation: The pull request does not provide sufficient documentation about the changes being made. It would be helpful to have comments or documentation added to the code to explain the purpose and usage of the new functions and methods.

   • Suggestion: Add comments or documentation to explain the purpose and usage of the new functions and methods.

2. Unused import: The "go.uber.org/zap" package is imported on line 6, but it is not used anywhere in the code. This can be considered as unnecessary code and should be removed.

   • Suggestion: Remove the unused import statement for "go.uber.org/zap".

3. Magic numbers: The IDXString function uses a switch statement to return a string representation of an index value. However, the meaning of these index values is not clear from the code alone. It would be better to use constants or enums to represent these index values and provide meaningful names for them.

   • Suggestion: Define constants or enums for the index values and use them in the IDXString function.

4. Inefficient string concatenation: In the ExportStats function, the zap.Int function is called multiple times to create fields for each index value. This can be inefficient as it creates a new string for each call. It would be more efficient to use a buffer or a string builder to concatenate the strings.

   • Suggestion: Use a buffer or a string builder to concatenate the strings in the ExportStats function.

5. Lack of error handling: The GlobalCheckpointDataFactory function does not handle any errors. If an error occurs, it is not returned or handled in any way. It would be better to handle errors and return them to the caller.

   • Suggestion: Add error handling code and return the error to the caller if an error occurs in the GlobalCheckpointDataFactory function.

Security Issues:
No security issues were identified in the provided code changes.