Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added some documentation and a watermark gauge #261

Merged
merged 8 commits into from
Aug 31, 2021

Conversation

Sanskar95
Copy link
Collaborator

No description provided.

src/lib.rs Outdated
Comment on lines 44 to 51
//! - `allocator_metrics`
//! - This enables recording metrics like total_bytes, bytes_remaining, allocation_counter
//! - It is to be noted that without the prometheus_exporter flag the metrics will flow to stdout.
//!
//! - `state_metrics`
//! - If this flag is enabled one can record state metrics like bytes_read, bytes_written which will be respective to the respective backend.
//! - One can also record checkpoint related metrics like size of last checkpoint.
//!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//! - `allocator_metrics`
//! - This enables recording metrics like total_bytes, bytes_remaining, allocation_counter
//! - It is to be noted that without the prometheus_exporter flag the metrics will flow to stdout.
//!
//! - `state_metrics`
//! - If this flag is enabled one can record state metrics like bytes_read, bytes_written which will be respective to the respective backend.
//! - One can also record checkpoint related metrics like size of last checkpoint.
//!
//! - `allocator_metrics`
//! - With this feature on, the runtime will record allocator metrics (e.g., total_bytes, bytes_remaining).
//! - `state_metrics`
//! - With this feature on, the runtime will record various state metrics (e.g., bytes in/out, checkpoint size).

Comment on lines 6 to 9
#[derive(Deserialize, Clone, Debug)]
pub enum HardwareCounter {
/// The type of hardware metric one wants to record.This is done while building the application and adding HardwareCounter
/// enum options to it.
Copy link
Member

Choose a reason for hiding this comment

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

What is there now is a rustdoc comment for the first Enum variant. It needs to go on top.

Suggested change
#[derive(Deserialize, Clone, Debug)]
pub enum HardwareCounter {
/// The type of hardware metric one wants to record.This is done while building the application and adding HardwareCounter
/// enum options to it.
/// An enum representing supported hardware counters
///
/// It is a wrapper around [Hardware] in order to support [Deserialize]
#[derive(Deserialize, Clone, Debug)]
pub enum HardwareCounter {

@Max-Meldrum
Copy link
Member

Missing rustdoc for PerfEvents

/// Configurable hardware counters that may be added to an Operator's config
///
///```no_run
/// let mut perf_events = PerfEvents::new();
/// perf_events.add(HardwareCounter::CpuCycles);
/// perf_events.add(HardwareCounter::BranchMisses);
///```
#[derive(Deserialize, Clone, Debug, Default)]
pub struct PerfEvents {
    pub counters: Vec<HardwareCounter>,
}

///```no_run
/// use arcon::prelude::*;
/// #[derive(Default)]
/// pub struct MyOperator;
Copy link
Member

Choose a reason for hiding this comment

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

Good idea to include a simple application. We can however use an already defined operator.

operator: Arc::new(|| Map::new(|x| x + 10))

Also, please make sure it's indented properly.

Comment on lines 5 to 8
/// An enum representing supported hardware counters with perf events as enum options
///
/// It is a wrapper around [Hardware] in order to support [Deserialize]
///
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// An enum representing supported hardware counters with perf events as enum options
///
/// It is a wrapper around [Hardware] in order to support [Deserialize]
///
/// An enum representing supported hardware counters with perf events as enum options
///
/// It is a wrapper around [Hardware] in order to support [Deserialize]

@Max-Meldrum
Copy link
Member

Closes #259

@Max-Meldrum Max-Meldrum merged commit bc5047d into cda-group:master Aug 31, 2021
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.

None yet

2 participants