Skip to content

Conversation

@maozhen520
Copy link
Contributor

Purpose of this pull request

This pull request addresses two issues identified in the HDFS checkpoint storage code:

  • Fix Double-Checked Locking: Ensures proper thread safety and lazy initialization.
  • Improve Logging: Enhances log messages for better debugging and clarity.

Does this PR introduce any user-facing change?

No

How was this patch tested?

This patch primarily involves minor changes to logging and the addition of the volatile keyword to a field. Given the nature of these changes, no new test cases were added. The existing test suite covers the functionality of the HdfsStorage class, and these changes do not introduce new logic that requires additional testing.
The changes made are:
Logging Improvements: Enhanced log messages for better clarity and debugging.
Volatile Keyword: Added the volatile keyword to ensure visibility of changes to the fs field across threads.
These modifications are unlikely to affect the existing behavior, and thus, the existing test cases are sufficient to validate the changes.

Check list

- 使用 volatile 关键字确保 HdfsStorage 实例的线程安全
- 优化 HdfsStorage 中的日志输出
@github-actions github-actions bot added the Zeta label Apr 18, 2025
@corgy-w
Copy link
Contributor

corgy-w commented Apr 20, 2025

Hi @maozhen520 , Please open ci according to the instructions

@hailin0 hailin0 requested a review from Copilot April 21, 2025 01:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a thread-safety issue with double-checked locking in the HDFS checkpoint storage by adding the volatile keyword and improves the logging format for enhanced clarity. Additionally, it refines the method signature in the FileConfiguration class to remove an unused parameter, ensuring consistency with its usage in HdfsStorage.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
HdfsFileStorageInstance.java Added the volatile keyword to HDFS_STORAGE for safe double-checked locking.
FileConfiguration.java Removed the unused parameter in getConfiguration(), aligning with its updated usage.
HdfsStorage.java Updated the logging message for clarity and adjusted configuration retrieval accordingly.

@maozhen520
Copy link
Contributor Author

Hi @maozhen520 , Please open ci according to the instructions

DONE.

@Hisoka-X Hisoka-X changed the title [FIX][CHECKPOINT-STORAGE] Fix Double-Checked Locking and Improve Logging in HdfsStorage [Fix][Zeta] Fix double-checked locking without volatile in HdfsStorage Apr 22, 2025
@hailin0 hailin0 merged commit 6f8c039 into apache:dev Apr 22, 2025
5 checks passed
hawk9821 pushed a commit to hawk9821/seatunnel that referenced this pull request Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants