Skip to content

Conversation

@chenBright
Copy link
Contributor

@chenBright chenBright commented Oct 26, 2025

What problem does this PR solve?

Issue Number: resolve #3069

Problem Summary:

MultiDimension get_stats returns a raw pointer, making it unsafe to delete bvar via delete_stats.

What is changed and the side effects?

Changed:

MultiDimension<BvarType, KeyType, true> get_stats supports returning shared bvar(std::shared_ptr), so bvar can be safely deleted via delete_stats.

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

Copy link

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 adds support for shared (thread-safe) bvar management in MultiDimension by introducing a template parameter Shared that controls whether get_stats returns a raw pointer or std::shared_ptr. When Shared=true, the shared pointer approach prevents use-after-free issues when deleting bvars concurrently.

Key changes:

  • Added Shared template parameter (defaulting to false for backward compatibility) to MultiDimension
  • Modified return types to use std::shared_ptr when Shared=true, raw pointers when Shared=false
  • Moved test code (MyStringView class and TestLabels function) from bvar_mvariable_unittest.cpp to bvar_multi_dimension_unittest.cpp and added new concurrency test

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
test/bvar_mvariable_unittest.cpp Removed MyStringView class definition, TestLabels function, and labels test; cleaned up unused variable
test/bvar_multi_dimension_unittest.cpp Added MyStringView class, TestLabels function, labels test, and new thread-safety test for shared bvar
src/bvar/multi_dimension_inl.h Updated all method signatures and implementations to support Shared template parameter; replaced nullptr with NULL for consistency
src/bvar/multi_dimension.h Added Shared template parameter, conditional typedef for value_ptr_type, and helper methods for creating/deleting values based on Shared flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@apache apache deleted a comment from Copilot AI Oct 26, 2025
@apache apache deleted a comment from Copilot AI Oct 26, 2025
@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 27, 2025

Note: The shared mbvar may be less performant than the non-shared version.

@chenBright
Copy link
Contributor Author

Note: The shared mbvar may be less performant than the non-shared version.

Documentation and comments have been updated.

@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 28, 2025

LGTM

@chenBright chenBright merged commit ef82950 into apache:master Nov 1, 2025
17 checks passed
@chenBright chenBright deleted the shared_mbvar branch November 1, 2025 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mvar clear_stats不是线程安全的

2 participants