-
Notifications
You must be signed in to change notification settings - Fork 138
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
renaming metrics #1224
renaming metrics #1224
Changes from 4 commits
29d6398
1b703f9
8439f45
30b1761
1a057eb
7e3b4de
42f2c8c
04e2106
569d8fc
62b0e7b
0b61450
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -212,7 +212,7 @@ public MLModelManager( | |
public void registerModelMeta(MLRegisterModelMetaInput mlRegisterModelMetaInput, ActionListener<String> listener) { | ||
try { | ||
FunctionName functionName = mlRegisterModelMetaInput.getFunctionName(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
mlStats.createCounterStatIfAbsent(functionName, REGISTER, ML_ACTION_REQUEST_COUNT).increment(); | ||
String modelGroupId = mlRegisterModelMetaInput.getModelGroupId(); | ||
if (Strings.isBlank(modelGroupId)) { | ||
|
@@ -322,8 +322,8 @@ public void registerMLModel(MLRegisterModelInput registerModelInput, MLTask mlTa | |
|
||
checkAndAddRunningTask(mlTask, maxRegisterTasksPerNode); | ||
try { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.createCounterStatIfAbsent(mlTask.getFunctionName(), REGISTER, ML_ACTION_REQUEST_COUNT).increment(); | ||
|
||
String modelGroupId = registerModelInput.getModelGroupId(); | ||
|
@@ -384,17 +384,17 @@ public void registerMLModel(MLRegisterModelInput registerModelInput, MLTask mlTa | |
} catch (Exception e) { | ||
handleException(registerModelInput.getFunctionName(), mlTask.getTaskId(), e); | ||
} finally { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
} | ||
} | ||
|
||
private void indexRemoteModel(MLRegisterModelInput registerModelInput, MLTask mlTask, String modelVersion) { | ||
String taskId = mlTask.getTaskId(); | ||
FunctionName functionName = mlTask.getFunctionName(); | ||
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
mlStats.createCounterStatIfAbsent(functionName, REGISTER, ML_ACTION_REQUEST_COUNT).increment(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is to track how many register requests on function level. By removing this, can we still track that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, because we are tracking this in the parent function |
||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
|
||
String modelName = registerModelInput.getModelName(); | ||
String version = modelVersion == null ? registerModelInput.getVersion() : modelVersion; | ||
|
@@ -444,7 +444,7 @@ private void indexRemoteModel(MLRegisterModelInput registerModelInput, MLTask ml | |
logException("Failed to upload model", e, log); | ||
handleException(functionName, taskId, e); | ||
} finally { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
} | ||
} | ||
|
||
|
@@ -462,9 +462,9 @@ private void registerModelFromUrl(MLRegisterModelInput registerModelInput, MLTas | |
String taskId = mlTask.getTaskId(); | ||
FunctionName functionName = mlTask.getFunctionName(); | ||
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
mlStats.createCounterStatIfAbsent(functionName, REGISTER, ML_ACTION_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
String modelName = registerModelInput.getModelName(); | ||
String version = modelVersion == null ? registerModelInput.getVersion() : modelVersion; | ||
String modelGroupId = registerModelInput.getModelGroupId(); | ||
|
@@ -510,7 +510,7 @@ private void registerModelFromUrl(MLRegisterModelInput registerModelInput, MLTas | |
logException("Failed to register model", e, log); | ||
handleException(functionName, taskId, e); | ||
} finally { | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT).increment(); | ||
} | ||
} | ||
|
||
|
@@ -693,7 +693,7 @@ private void handleException(FunctionName functionName, String taskId, Exception | |
&& !(e instanceof MLResourceNotFoundException) | ||
&& !(e instanceof IllegalArgumentException)) { | ||
mlStats.createCounterStatIfAbsent(functionName, REGISTER, MLActionLevelStat.ML_ACTION_FAILURE_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_FAILURE_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_FAILURE_COUNT).increment(); | ||
} | ||
Map<String, Object> updated = ImmutableMap.of(ERROR_FIELD, MLExceptionUtils.getRootCauseMessage(e), STATE_FIELD, FAILED); | ||
mlTaskManager.updateMLTask(taskId, updated, TIMEOUT_IN_MILLIS, true); | ||
|
@@ -718,7 +718,7 @@ public void deployModel( | |
ActionListener<String> listener | ||
) { | ||
mlStats.createCounterStatIfAbsent(functionName, ActionName.DEPLOY, ML_ACTION_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
List<String> workerNodes = mlTask.getWorkerNodes(); | ||
if (modelCacheHelper.isModelDeployed(modelId)) { | ||
if (workerNodes != null && workerNodes.size() > 0) { | ||
|
@@ -800,7 +800,7 @@ public void deployModel( | |
MLExecutable mlExecutable = mlEngine.deployExecute(mlModel, params); | ||
try { | ||
modelCacheHelper.setMLExecutor(modelId, mlExecutable); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT).increment(); | ||
modelCacheHelper.setModelState(modelId, MLModelState.DEPLOYED); | ||
listener.onResponse("successful"); | ||
} catch (Exception e) { | ||
|
@@ -813,7 +813,7 @@ public void deployModel( | |
Predictable predictable = mlEngine.deploy(mlModel, params); | ||
try { | ||
modelCacheHelper.setPredictor(modelId, predictable); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT).increment(); | ||
modelCacheHelper.setModelState(modelId, MLModelState.DEPLOYED); | ||
Long modelContentSizeInBytes = mlModel.getModelContentSizeInBytes(); | ||
long contentSize = modelContentSizeInBytes == null | ||
|
@@ -846,7 +846,7 @@ private void handleDeployModelException(String modelId, FunctionName functionNam | |
&& !(e instanceof MLResourceNotFoundException) | ||
&& !(e instanceof IllegalArgumentException)) { | ||
mlStats.createCounterStatIfAbsent(functionName, ActionName.DEPLOY, MLActionLevelStat.ML_ACTION_FAILURE_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_FAILURE_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_FAILURE_COUNT).increment(); | ||
} | ||
removeModel(modelId); | ||
listener.onFailure(e); | ||
|
@@ -855,7 +855,7 @@ private void handleDeployModelException(String modelId, FunctionName functionNam | |
private void setupPredictable(String modelId, MLModel mlModel, Map<String, Object> params) { | ||
Predictable predictable = mlEngine.deploy(mlModel, params); | ||
modelCacheHelper.setPredictor(modelId, predictable); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT).increment(); | ||
modelCacheHelper.setModelState(modelId, MLModelState.DEPLOYED); | ||
} | ||
|
||
|
@@ -1056,8 +1056,8 @@ public synchronized Map<String, String> undeployModel(String[] modelIds) { | |
for (String modelId : modelIds) { | ||
if (modelCacheHelper.isModelDeployed(modelId)) { | ||
modelUndeployStatus.put(modelId, UNDEPLOYED); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT).decrement(); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT).increment(); | ||
mlStats.getStat(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT).decrement(); | ||
mlStats.getStat(MLNodeLevelStat.ML_REQUEST_COUNT).increment(); | ||
mlStats | ||
.createCounterStatIfAbsent(getModelFunctionName(modelId), ActionName.UNDEPLOY, ML_ACTION_REQUEST_COUNT) | ||
.increment(); | ||
|
@@ -1070,7 +1070,7 @@ public synchronized Map<String, String> undeployModel(String[] modelIds) { | |
log.debug("undeploy all models {}", Arrays.toString(getLocalDeployedModels())); | ||
for (String modelId : getLocalDeployedModels()) { | ||
modelUndeployStatus.put(modelId, UNDEPLOYED); | ||
mlStats.getStat(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT).decrement(); | ||
mlStats.getStat(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT).decrement(); | ||
mlStats.createCounterStatIfAbsent(getModelFunctionName(modelId), ActionName.UNDEPLOY, ML_ACTION_REQUEST_COUNT).increment(); | ||
removeModel(modelId); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,11 +305,11 @@ public Collection<Object> createComponents( | |
stats.put(MLClusterLevelStat.ML_MODEL_COUNT, new MLStat<>(true, new CounterSupplier())); | ||
stats.put(MLClusterLevelStat.ML_CONNECTOR_COUNT, new MLStat<>(true, new CounterSupplier())); | ||
// node level stats | ||
stats.put(MLNodeLevelStat.ML_NODE_EXECUTING_TASK_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_NODE_TOTAL_REQUEST_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_NODE_TOTAL_FAILURE_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_NODE_TOTAL_MODEL_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_NODE_TOTAL_CIRCUIT_BREAKER_TRIGGER_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_EXECUTING_TASK_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_REQUEST_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_FAILURE_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
stats.put(MLNodeLevelStat.ML_TOTAL_MODEL_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see other name change following the pattern by removing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the cluster level we have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After further discussion, as |
||
stats.put(MLNodeLevelStat.ML_CIRCUIT_BREAKER_TRIGGER_COUNT, new MLStat<>(false, new CounterSupplier())); | ||
this.mlStats = new MLStats(stats); | ||
|
||
mlIndicesHandler = new MLIndicesHandler(clusterService, client); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
import java.util.Locale; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.Set; | ||
import java.util.stream.Collectors; | ||
|
||
import org.opensearch.client.node.NodeClient; | ||
|
@@ -148,6 +149,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli | |
} | ||
|
||
MLStatsInput createMlStatsInputFromRequestParams(RestRequest request) { | ||
|
||
Set<String> mlNodeStatNames = EnumSet.allOf(MLNodeLevelStat.class).stream() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we construct a new Set for every request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can construct the new set in the class initialization. Let me do that. |
||
.map(stat -> stat.name()) | ||
.collect(Collectors.toSet()); | ||
MLStatsInput mlStatsInput = new MLStatsInput(); | ||
Optional<String[]> nodeIds = splitCommaSeparatedParam(request, "nodeId"); | ||
if (nodeIds.isPresent()) { | ||
|
@@ -158,7 +163,7 @@ MLStatsInput createMlStatsInputFromRequestParams(RestRequest request) { | |
for (String state : stats.get()) { | ||
state = state.toUpperCase(Locale.ROOT); | ||
// only support cluster and node level stats for bwc | ||
if (state.startsWith("ML_NODE")) { | ||
if (mlNodeStatNames.contains(state)) { | ||
mlStatsInput.getNodeLevelStats().add(MLNodeLevelStat.from(state)); | ||
} else { | ||
mlStatsInput.getClusterLevelStats().add(MLClusterLevelStat.from(state)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are already counting this in the
registerMLModel
function in MLModelManager class