Skip to content

Commit

Permalink
Merge pull request #1793 from chutten/bug1729723-exposePersistPingLif…
Browse files Browse the repository at this point in the history
…etime

Bug1729723 expose persist ping lifetime
  • Loading branch information
badboy authored Sep 16, 2021
2 parents c8471be + d2b75bd commit 7a7dc96
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 4 deletions.
29 changes: 25 additions & 4 deletions glean-core/rlb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,7 @@ fn initialize_internal(
Some(init_handle)
}

/// Shuts down Glean.
///
/// This currently only attempts to shut down the
/// internal dispatcher.
/// Shuts down Glean in an orderly fashion.
pub fn shutdown() {
if global_glean().is_none() {
log::warn!("Shutdown called before Glean is initialized");
Expand All @@ -379,6 +376,13 @@ pub fn shutdown() {
if let Err(e) = dispatcher::shutdown() {
log::error!("Can't shutdown dispatcher thread: {:?}", e);
}

// Be sure to call this _after_ draining the dispatcher
crate::with_glean(|glean| {
if let Err(e) = glean.persist_ping_lifetime_data() {
log::error!("Can't persist ping lifetime data: {:?}", e);
}
});
}

/// Unblock the global dispatcher to start processing queued tasks.
Expand Down Expand Up @@ -784,5 +788,22 @@ pub fn get_timestamp_ms() -> u64 {
glean_core::get_timestamp_ms()
}

/// Asks the database to persist ping-lifetime data to disk. Probably expensive to call.
/// Only has effect when Glean is configured with `delay_ping_lifetime_io: true`.
/// If Glean hasn't been initialized this will dispatch and return Ok(()),
/// otherwise it will block until the persist is done and return its Result.
pub fn persist_ping_lifetime_data() -> Result<()> {
if !was_initialize_called() {
crate::launch_with_glean(|glean| {
// This is async, we can't get the Error back to the caller.
let _ = glean.persist_ping_lifetime_data();
});
Ok(())
} else {
block_on_dispatcher();
with_glean(|glean| glean.persist_ping_lifetime_data())
}
}

#[cfg(test)]
mod test;
96 changes: 96 additions & 0 deletions glean-core/rlb/tests/persist_ping_lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

//! This integration test should model how the RLB is used when embedded in another Rust application
//! (e.g. FOG/Firefox Desktop).
//!
//! We write a single test scenario per file to avoid any state keeping across runs
//! (different files run as different processes).
mod common;

use glean::{ClientInfoMetrics, Configuration};
use std::path::PathBuf;

/// Some user metrics.
mod metrics {
use glean::private::*;
use glean::Lifetime;
use glean_core::CommonMetricData;
use once_cell::sync::Lazy;

#[allow(non_upper_case_globals)]
pub static boo: Lazy<BooleanMetric> = Lazy::new(|| {
BooleanMetric::new(CommonMetricData {
name: "boo".into(),
category: "sample".into(),
send_in_pings: vec!["validation".into()],
lifetime: Lifetime::Ping,
disabled: false,
..Default::default()
})
});
}

fn cfg_new(tmpname: PathBuf) -> Configuration {
Configuration {
data_path: tmpname,
application_id: "firefox-desktop".into(),
upload_enabled: true,
max_events: None,
delay_ping_lifetime_io: true,
channel: Some("testing".into()),
server_endpoint: Some("invalid-test-host".into()),
uploader: None,
use_core_mps: false,
}
}

/// Test scenario: Are ping-lifetime data persisted on shutdown when delayed?
///
/// delay_ping_lifetime_io: true has Glean put "ping"-lifetime data in-memory
/// instead of the db. Ensure that, on orderly shutdowns, we correctly persist
/// these in-memory data to the db.
#[test]
fn delayed_ping_data() {
common::enable_test_logging();

metrics::boo.set(true);

// Create a custom configuration to delay ping-lifetime io
let dir = tempfile::tempdir().unwrap();
let tmpname = dir.path().to_path_buf();

common::initialize(cfg_new(tmpname.clone()));

assert!(
metrics::boo.test_get_value(None).unwrap(),
"Data should be present. Doesn't mean it's persisted, though."
);

glean::test_reset_glean(
cfg_new(tmpname.clone()),
ClientInfoMetrics::unknown(),
false,
);

assert_eq!(
None,
metrics::boo.test_get_value(None),
"Data should not have made it to disk on unclean shutdown."
);
metrics::boo.set(true); // Let's try again

// This time, let's shut down cleanly
glean::shutdown();

// Now when we init, we should get the persisted data
glean::test_reset_glean(cfg_new(tmpname), ClientInfoMetrics::unknown(), false);
assert!(
metrics::boo.test_get_value(None).unwrap(),
"Data must be persisted between clean shutdown and init!"
);

glean::shutdown(); // Cleanly shut down at the end of the test.
}

0 comments on commit 7a7dc96

Please sign in to comment.