Skip to content

Commit

Permalink
Instrument scanning of pending pings dir on init
Browse files Browse the repository at this point in the history
  • Loading branch information
brizental committed Aug 3, 2020
1 parent a3ed4cb commit 5e55a0b
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 13 deletions.
38 changes: 38 additions & 0 deletions glean-core/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -391,3 +391,41 @@ glean.upload:
expires: never
no_lint:
- COMMON_PREFIX

pending_pings_directory_size:
type: memory_distribution
description: >
The size of the pending pings directory upon initialization of Glean.
This does not include the size of the deletion request pings directory.
memory_unit: kilobyte
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1601550
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1601550#c3
data_sensitivity:
- technical
notification_emails:
- glean-team@mozilla.com
expires: never
no_lint:
- COMMON_PREFIX

deleted_pending_pings_after_quota_hit:
type: counter
description: >
The number of pings deleted after the quota
for the size of the pending pings directory is hit.
Since quota is only calculated for the pending pings directory,
and deleteion request ping live in a different directory,
deletion request pings are never deleted.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1601550
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1601550#c3
data_sensitivity:
- technical
notification_emails:
- glean-team@mozilla.com
expires: never
no_lint:
- COMMON_PREFIX
23 changes: 23 additions & 0 deletions glean-core/src/internal_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ impl CoreMetrics {
pub struct UploadMetrics {
pub ping_upload_failure: LabeledMetric<CounterMetric>,
pub discarded_exceeding_pings_size: MemoryDistributionMetric,
pub pending_pings_directory_size: MemoryDistributionMetric,
pub deleted_pending_pings_after_quota_hit: CounterMetric,
}

impl UploadMetrics {
Expand Down Expand Up @@ -85,6 +87,27 @@ impl UploadMetrics {
},
MemoryUnit::Kilobyte,
),

pending_pings_directory_size: MemoryDistributionMetric::new(
CommonMetricData {
name: "pending_pings_directory_size".into(),
category: "glean.upload".into(),
send_in_pings: vec!["metrics".into()],
lifetime: Lifetime::Ping,
disabled: false,
dynamic_label: None,
},
MemoryUnit::Kilobyte,
),

deleted_pending_pings_after_quota_hit: CounterMetric::new(CommonMetricData {
name: "deleted_pending_pings_after_quota_hit".into(),
category: "glean.upload".into(),
send_in_pings: vec!["metrics".into()],
lifetime: Lifetime::Ping,
disabled: false,
dynamic_label: None,
}),
}
}
}
24 changes: 17 additions & 7 deletions glean-core/src/upload/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl PingPayloadsByDirectory {
}
}

/// G et the file name from a path as a &str.
/// Get the file name from a path as a &str.
///
/// # Panics
///
Expand Down Expand Up @@ -102,32 +102,42 @@ impl PingDirectoryManager {

/// Attempts to delete a ping file.
///
/// ## Arguments
/// # Arguments
///
/// * `uuid` - The UUID of the ping file to be deleted
///
/// ## Panics
/// # Return value
///
/// `bool` - `true` if file was deleted succesfully and `false` otherwise.
///
/// # Panics
///
/// Won't panic if unable to delete the file.
pub fn delete_file(&self, uuid: &str) {
pub fn delete_file(&self, uuid: &str) -> bool {
let path = match self.get_file_path(uuid) {
Some(path) => path,
None => {
log::error!("Cannot find ping file to delete {}", uuid);
return;
return false;
}
};

match fs::remove_file(&path) {
Err(e) => log::error!("Error deleting file {}. {}", path.display(), e),
Err(e) => {
log::error!("Error deleting file {}. {}", path.display(), e);
return false;
}
_ => log::info!("File was deleted {}", path.display()),
};

true
}

/// Reads a ping file and returns the data from it.
///
/// If the file is not properly formatted, it will be deleted and `None` will be returned.
///
/// ## Arguments
/// # Arguments
///
/// * `document_id` - The UUID of the ping file to be processed
pub fn process_file(&self, document_id: &str) -> Option<PingPayload> {
Expand Down
17 changes: 11 additions & 6 deletions glean-core/src/upload/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,21 +311,26 @@ impl PingUploadManager {
// Enqueue pending pings until we reach quota,
// after that delete outstanding pings.
let pending_pings = cached_pings.pending_pings.drain(..);
let mut total: usize = 0;
let mut pending_pings_directory_size: usize = 0;
let mut enqueueing = true;
for (metadata, (document_id, path, body, headers)) in pending_pings {
total += metadata.len() as usize;
println!("{}", total);
if total > quota {
pending_pings_directory_size += metadata.len() as usize;
if pending_pings_directory_size > quota {
enqueueing = false;
}

if enqueueing {
self.enqueue_ping(glean, &document_id, &path, &body, headers);
} else {
self.directory_manager.delete_file(&document_id);
} else if self.directory_manager.delete_file(&document_id) {
self.upload_metrics
.deleted_pending_pings_after_quota_hit
.add(glean, 1);
}
}

self.upload_metrics
.pending_pings_directory_size
.accumulate(glean, pending_pings_directory_size as u64);
}

/// Adds rate limiting capability to this upload manager. The rate limiter
Expand Down

0 comments on commit 5e55a0b

Please sign in to comment.