Fix notification event stream cleanup, add bounded send concurrency, and reduce overhead (#1224)

This commit is contained in:
houseme
2025-12-22 00:57:05 +08:00
committed by GitHub
parent 1c51e204ab
commit 08f1a31f3f
20 changed files with 921 additions and 169 deletions
+65 -31
View File
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use crate::notification_system_subscriber::NotificationSystemSubscriberView;
use crate::{
Event, error::NotificationError, notifier::EventNotifier, registry::TargetRegistry, rules::BucketNotificationConfig, stream,
};
@@ -104,6 +105,8 @@ pub struct NotificationSystem {
concurrency_limiter: Arc<Semaphore>,
/// Monitoring indicators
metrics: Arc<NotificationMetrics>,
/// Subscriber view
subscriber_view: NotificationSystemSubscriberView,
}
impl NotificationSystem {
@@ -112,6 +115,7 @@ impl NotificationSystem {
let concurrency_limiter =
rustfs_utils::get_env_usize(ENV_NOTIFY_TARGET_STREAM_CONCURRENCY, DEFAULT_NOTIFY_TARGET_STREAM_CONCURRENCY);
NotificationSystem {
subscriber_view: NotificationSystemSubscriberView::new(),
notifier: Arc::new(EventNotifier::new()),
registry: Arc::new(TargetRegistry::new()),
config: Arc::new(RwLock::new(config)),
@@ -188,8 +192,11 @@ impl NotificationSystem {
}
/// Checks if there are active subscribers for the given bucket and event name.
pub async fn has_subscriber(&self, bucket: &str, event_name: &EventName) -> bool {
self.notifier.has_subscriber(bucket, event_name).await
pub async fn has_subscriber(&self, bucket: &str, event: &EventName) -> bool {
if !self.subscriber_view.has_subscriber(bucket, event) {
return false;
}
self.notifier.has_subscriber(bucket, event).await
}
async fn update_config_and_reload<F>(&self, mut modifier: F) -> Result<(), NotificationError>
@@ -236,15 +243,18 @@ impl NotificationSystem {
pub async fn remove_target(&self, target_id: &TargetID, target_type: &str) -> Result<(), NotificationError> {
info!("Attempting to remove target: {}", target_id);
let ttype = target_type.to_lowercase();
let tname = target_id.name.to_lowercase();
self.update_config_and_reload(|config| {
let mut changed = false;
if let Some(targets_of_type) = config.0.get_mut(target_type) {
if targets_of_type.remove(&target_id.name).is_some() {
if let Some(targets_of_type) = config.0.get_mut(&ttype) {
if targets_of_type.remove(&tname).is_some() {
info!("Removed target {} from configuration", target_id);
changed = true;
}
if targets_of_type.is_empty() {
config.0.remove(target_type);
config.0.remove(&ttype);
}
}
if !changed {
@@ -269,20 +279,24 @@ impl NotificationSystem {
/// If the target configuration is invalid, it returns Err(NotificationError::Configuration).
pub async fn set_target_config(&self, target_type: &str, target_name: &str, kvs: KVS) -> Result<(), NotificationError> {
info!("Setting config for target {} of type {}", target_name, target_type);
let ttype = target_type.to_lowercase();
let tname = target_name.to_lowercase();
self.update_config_and_reload(|config| {
config
.0
.entry(target_type.to_lowercase())
.or_default()
.insert(target_name.to_lowercase(), kvs.clone());
config.0.entry(ttype.clone()).or_default().insert(tname.clone(), kvs.clone());
true // The configuration is always modified
})
.await
}
/// Removes all notification configurations for a bucket.
pub async fn remove_bucket_notification_config(&self, bucket_name: &str) {
self.notifier.remove_rules_map(bucket_name).await;
/// If the configuration is successfully removed, the entire notification system will be automatically reloaded.
///
/// # Arguments
/// * `bucket` - The name of the bucket whose notification configuration is to be removed.
///
pub async fn remove_bucket_notification_config(&self, bucket: &str) {
self.subscriber_view.clear_bucket(bucket);
self.notifier.remove_rules_map(bucket).await;
}
/// Removes a Target configuration.
@@ -299,11 +313,28 @@ impl NotificationSystem {
/// If the target configuration does not exist, it returns Ok(()) without making any changes.
pub async fn remove_target_config(&self, target_type: &str, target_name: &str) -> Result<(), NotificationError> {
info!("Removing config for target {} of type {}", target_name, target_type);
let ttype = target_type.to_lowercase();
let tname = target_name.to_lowercase();
let target_id = TargetID {
id: tname.clone(),
name: ttype.clone(),
};
// Deletion is prohibited if bucket rules refer to it
if self.notifier.is_target_bound_to_any_bucket(&target_id).await {
return Err(NotificationError::Configuration(format!(
"Target is still bound to bucket rules and deletion is prohibited: type={} name={}",
ttype, tname
)));
}
let config_result = self
.update_config_and_reload(|config| {
let mut changed = false;
if let Some(targets) = config.0.get_mut(&target_type.to_lowercase()) {
if targets.remove(&target_name.to_lowercase()).is_some() {
if let Some(targets) = config.0.get_mut(&ttype) {
if targets.remove(&tname).is_some() {
changed = true;
}
if targets.is_empty() {
@@ -319,8 +350,6 @@ impl NotificationSystem {
.await;
if config_result.is_ok() {
let target_id = TargetID::new(target_name.to_string(), target_type.to_string());
// Remove from target list
let target_list = self.notifier.target_list();
let mut target_list_guard = target_list.write().await;
@@ -358,6 +387,9 @@ impl NotificationSystem {
let _ = cancel_tx.send(()).await;
}
// Clear the target_list and ensure that reload is a replacement reconstruction (solve the target_list len unchanged/residual problem)
self.notifier.remove_all_bucket_targets().await;
// Update the config
self.update_config(new_config.clone()).await;
@@ -388,15 +420,16 @@ impl NotificationSystem {
// The storage of the cloned target and the target itself
let store_clone = store.boxed_clone();
let target_box = target.clone_dyn();
let target_arc = Arc::from(target_box);
// Add a reference to the monitoring metrics
let metrics = self.metrics.clone();
let semaphore = self.concurrency_limiter.clone();
// let target_box = target.clone_dyn();
let target_arc = Arc::from(target.clone_dyn());
// Encapsulated enhanced version of start_event_stream
let cancel_tx = self.enhanced_start_event_stream(store_clone, target_arc, metrics, semaphore);
let cancel_tx = self.enhanced_start_event_stream(
store_clone,
target_arc,
self.metrics.clone(),
self.concurrency_limiter.clone(),
);
// Start event stream processing and save cancel sender
// let cancel_tx = start_event_stream(store_clone, target_clone);
@@ -423,17 +456,18 @@ impl NotificationSystem {
/// Loads the bucket notification configuration
pub async fn load_bucket_notification_config(
&self,
bucket_name: &str,
config: &BucketNotificationConfig,
bucket: &str,
cfg: &BucketNotificationConfig,
) -> Result<(), NotificationError> {
let arn_list = self.notifier.get_arn_list(&config.region).await;
self.subscriber_view.apply_bucket_config(bucket, cfg);
let arn_list = self.notifier.get_arn_list(&cfg.region).await;
if arn_list.is_empty() {
return Err(NotificationError::Configuration("No targets configured".to_string()));
}
info!("Available ARNs: {:?}", arn_list);
// Validate the configuration against the available ARNs
if let Err(e) = config.validate(&config.region, &arn_list) {
debug!("Bucket notification config validation region:{} failed: {}", &config.region, e);
if let Err(e) = cfg.validate(&cfg.region, &arn_list) {
debug!("Bucket notification config validation region:{} failed: {}", &cfg.region, e);
if !e.to_string().contains("ARN not found") {
return Err(NotificationError::BucketNotification(e.to_string()));
} else {
@@ -441,9 +475,9 @@ impl NotificationSystem {
}
}
let rules_map = config.get_rules_map();
self.notifier.add_rules_map(bucket_name, rules_map.clone()).await;
info!("Loaded notification config for bucket: {}", bucket_name);
let rules_map = cfg.get_rules_map();
self.notifier.add_rules_map(bucket, rules_map.clone()).await;
info!("Loaded notification config for bucket: {}", bucket);
Ok(())
}