alamb commented on code in PR #18000:
URL: https://github.com/apache/datafusion/pull/18000#discussion_r2417725098
##########
datafusion-cli/src/object_storage/instrumented.rs:
##########
@@ -15,36 +15,90 @@
// specific language governing permissions and limitations
// under the License.
-use std::{fmt, sync::Arc};
+use std::{
+ fmt,
+ str::FromStr,
+ sync::{
+ atomic::{AtomicU8, Ordering},
+ Arc,
+ },
+};
use async_trait::async_trait;
-use datafusion::execution::object_store::ObjectStoreRegistry;
+use datafusion::{error::DataFusionError,
execution::object_store::ObjectStoreRegistry};
use futures::stream::BoxStream;
use object_store::{
path::Path, GetOptions, GetResult, ListResult, MultipartUpload, ObjectMeta,
ObjectStore, PutMultipartOptions, PutOptions, PutPayload, PutResult,
Result,
};
use url::Url;
+/// The profiling mode to use for an [`ObjectStore`] instance that has been
instrumented to collect
+/// profiling data. Collecting profiling data will have a small negative
impact on both CPU and
+/// memory usage. Default is `Disabled`
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+pub enum InstrumentedObjectStoreMode {
+ /// Disable collection of profiling data
+ #[default]
+ Disabled,
+ /// Enable collection of profiling data
+ Enabled,
+}
+
+impl fmt::Display for InstrumentedObjectStoreMode {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ write!(f, "{:?}", self)
+ }
+}
+
+impl FromStr for InstrumentedObjectStoreMode {
+ type Err = DataFusionError;
+
+ fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
+ match s.to_lowercase().as_str() {
+ "disabled" => Ok(Self::Disabled),
+ "enabled" => Ok(Self::Enabled),
+ _ => Err(DataFusionError::Execution(format!("Unrecognized mode
{s}"))),
+ }
+ }
+}
+
+impl From<u8> for InstrumentedObjectStoreMode {
+ fn from(value: u8) -> Self {
+ match value {
+ 1 => InstrumentedObjectStoreMode::Enabled,
+ _ => InstrumentedObjectStoreMode::Disabled,
+ }
+ }
+}
+
/// Wrapped [`ObjectStore`] instances that record information for reporting on
the usage of the
/// inner [`ObjectStore`]
#[derive(Debug)]
struct InstrumentedObjectStore {
inner: Arc<dyn ObjectStore>,
+ instrument_mode: AtomicU8,
Review Comment:
maybe an `AtomicBool` would make sense given it only has two states 🤔
Minor detail however
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
