anujmodi2021 commented on code in PR #8056:
URL: https://github.com/apache/hadoop/pull/8056#discussion_r2567424700
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManagerV2.java:
##########
@@ -105,14 +109,33 @@ public final class ReadBufferManagerV2 extends
ReadBufferManager {
private static AtomicBoolean isConfigured = new AtomicBoolean(false);
+ /* Metrics collector for monitoring the performance of the ABFS read thread
pool. */
+ private final AbfsReadResourceUtilizationMetrics readThreadPoolMetrics;
+ /* The ABFSCounters instance for updating read buffer manager related
metrics. */
+ private final AbfsCounters abfsCounters;
+ /* Tracks the last scale direction applied, or empty if none. */
+ private volatile String lastScaleDirection = EMPTY_STRING;
+ /* Maximum CPU utilization observed during the monitoring interval. */
+ private volatile double maxJvmCpuUtilization = 0.0;
+
/**
* Private constructor to prevent instantiation as this needs to be
singleton.
+ *
+ * @param abfsCounters the {@link AbfsCounters} used for managing read
operations.
*/
- private ReadBufferManagerV2() {
+ private ReadBufferManagerV2(AbfsCounters abfsCounters) {
Review Comment:
This constructor will be called only once for all the FSs
Are abfsCounters and readThreadPoolMetrics also singleton?
What if these are different for different Filesystems?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManagerV2.java:
##########
@@ -105,14 +109,33 @@ public final class ReadBufferManagerV2 extends
ReadBufferManager {
private static AtomicBoolean isConfigured = new AtomicBoolean(false);
+ /* Metrics collector for monitoring the performance of the ABFS read thread
pool. */
+ private final AbfsReadResourceUtilizationMetrics readThreadPoolMetrics;
+ /* The ABFSCounters instance for updating read buffer manager related
metrics. */
+ private final AbfsCounters abfsCounters;
+ /* Tracks the last scale direction applied, or empty if none. */
+ private volatile String lastScaleDirection = EMPTY_STRING;
+ /* Maximum CPU utilization observed during the monitoring interval. */
+ private volatile double maxJvmCpuUtilization = 0.0;
+
/**
* Private constructor to prevent instantiation as this needs to be
singleton.
+ *
+ * @param abfsCounters the {@link AbfsCounters} used for managing read
operations.
*/
- private ReadBufferManagerV2() {
+ private ReadBufferManagerV2(AbfsCounters abfsCounters) {
+ this.abfsCounters = abfsCounters;
+ readThreadPoolMetrics =
abfsCounters.getAbfsReadResourceUtilizationMetrics();
printTraceLog("Creating Read Buffer Manager V2 with HADOOP-18546 patch");
}
- static ReadBufferManagerV2 getBufferManager() {
+ /**
+ * Returns the singleton instance of {@code ReadBufferManagerV2} for the
given ABFS client.
Review Comment:
Comment still need to be fixed
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManagerV2.java:
##########
@@ -104,14 +109,39 @@ public final class ReadBufferManagerV2 extends
ReadBufferManager {
private static AtomicBoolean isConfigured = new AtomicBoolean(false);
+ /* Metrics collector for monitoring the performance of the ABFS read thread
pool. */
+ private final AbfsReadThreadPoolMetrics readThreadPoolMetrics;
+
+ /* Last recorded CPU time used for computing CPU utilization deltas. */
+ private static long lastCpuTime = 0;
+
+ /* Last recorded system time used for utilization calculations. */
+ private static long lastTime = 0;
+
+ private final AbfsClient abfsClient;
+ /* Tracks the last scale direction applied, or empty if none. */
+ private volatile String lastScaleDirection = EMPTY_STRING;
+ /* Maximum CPU utilization observed during the monitoring interval. */
+ private volatile double maxCpuUtilization = 0.0;
+
/**
- * Private constructor to prevent instantiation as this needs to be
singleton.
+ * Initializes a new instance of {@code ReadBufferManagerV2} for the given
ABFS client.
Review Comment:
This seems pending
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/utils/TracingHeaderVersion.java:
##########
@@ -38,7 +38,16 @@ public enum TracingHeaderVersion {
* :primaryRequestId:streamId:opType:retryHeader:ingressHandler
*
:position:operatedBlobCount:operationSpecificHeader:httpOperationHeader
*/
- V1("v1", 13);
+ V1("v1", 13),
+ /**
+ * Version 1 of the tracing header, which includes a version prefix and has
13 permanent fields.
+ * This version is used for the current tracing header schema.
+ * Schema: version:clientCorrelationId:clientRequestId:fileSystemId
+ * :primaryRequestId:streamId:opType:retryHeader:ingressHandler
+ *
:position:operatedBlobCount:operationSpecificHeader:httpOperationHeader
+ * :networkLibrary:operationMetrics
Review Comment:
Earlier it was 13 right?
We are only adding one more in this PR.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java:
##########
@@ -242,6 +253,18 @@ public AzureIngressHandler getIngressHandler() {
private volatile boolean switchCompleted = false;
+ /**
+ * Starts CPU monitoring in the thread pool size manager if it
+ * is initialized and not already monitoring.
+ */
+ private void initializeMonitoringIfNeeded() {
+ if (poolSizeManager != null && !poolSizeManager.isMonitoringStarted()) {
+ synchronized (this) {
+ poolSizeManager.startCPUMonitoring();
+ }
+ }
+ }
Review Comment:
Was that supposed to be taken separately?
--
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]
