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.

Sign up for GitHub

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

Fix failures due to delete experiment #1437

Open
wants to merge 4 commits into
base: mvp_demo
Choose a base branch
from
Open

Fix failures due to delete experiment #1437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a39ffde
modify delete experiment to include rm flag
khansaad Dec 20, 2024
4c00793
fix PR check failure
khansaad Dec 20, 2024
e14f88f
update table names to fix deletion issue and corresponding test changes
khansaad Dec 20, 2024
6cf5dd0
modify delete experiment test function in the test files
khansaad Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/main/java/com/autotune/analyzer/services/CreateExperiment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
protected void doDelete(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
Map<String, KruizeObject> mKruizeExperimentMap = new ConcurrentHashMap<String, KruizeObject>();
String inputData = "";
String rm = request.getParameter(AnalyzerConstants.ServiceConstants.RM);
boolean rmTable = false;
Copy link
Member

Choose a reason for hiding this comment

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

As the if block below is used to just match the value and set the rmTable, we could simplify it with

// Check if rm is not null and set to true
boolean rmTable = (null != rm && AnalyzerConstants.BooleanString.TRUE.equalsIgnoreCase(rm.trim()));

if (null != rm
&& !rm.isEmpty()
&& rm.equalsIgnoreCase(AnalyzerConstants.BooleanString.TRUE)
) {
rmTable = true;
}
try {
inputData = request.getReader().lines().collect(Collectors.joining());
CreateExperimentAPIObject[] createExperimentAPIObjects = new Gson().fromJson(inputData, CreateExperimentAPIObject[].class);
Expand All @@ -180,7 +188,11 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response
} else {
for (CreateExperimentAPIObject ko : createExperimentAPIObjects) {
try {
new ExperimentDBService().loadExperimentFromDBByName(mKruizeExperimentMap, ko.getExperimentName());
if(rmTable) {
new ExperimentDBService().loadExperimentFromDBByName(mKruizeExperimentMap, ko.getExperimentName());
} else {
new ExperimentDBService().loadLMExperimentFromDBByName(mKruizeExperimentMap, ko.getExperimentName());
}
} catch (Exception e) {
LOGGER.error("Loading saved experiment {} failed: {} ", ko.getExperimentName(), e.getMessage());
}
Expand All @@ -189,7 +201,12 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response
for (CreateExperimentAPIObject ko : createExperimentAPIObjects) {
String expName = ko.getExperimentName();
if (!mKruizeExperimentMap.isEmpty() && mKruizeExperimentMap.containsKey(expName)) {
ValidationOutputData validationOutputData = new ExperimentDAOImpl().deleteKruizeExperimentEntryByName(expName);
ValidationOutputData validationOutputData;
if(rmTable) {
validationOutputData = new ExperimentDAOImpl().deleteKruizeExperimentEntryByName(expName);
} else {
validationOutputData = new ExperimentDAOImpl().deleteKruizeLMExperimentEntryByName(expName);
}
if (validationOutputData.isSuccess()) {
mKruizeExperimentMap.remove(ko.getExperimentName());
} else {
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/com/autotune/database/dao/ExperimentDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ public interface ExperimentDAO {
// Update experiment status
public boolean updateExperimentStatus(KruizeObject kruizeObject, AnalyzerConstants.ExperimentStatus status);

// Delete experiment
// Delete RM experiment
public ValidationOutputData deleteKruizeExperimentEntryByName(String experimentName);

// Delete LM experiment
public ValidationOutputData deleteKruizeLMExperimentEntryByName(String experimentName);

// If Kruize object restarts load all experiment which are in inprogress
public List<KruizeExperimentEntry> loadAllExperiments() throws Exception;

Expand Down
44 changes: 44 additions & 0 deletions src/main/java/com/autotune/database/dao/ExperimentDAOImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,50 @@ public ValidationOutputData deleteKruizeExperimentEntryByName(String experimentN
return validationOutputData;
}


/**
* Delete an experiment with the name experimentName
* This deletes the experiment from two tables kruize_lm_experiments, kruize_recommendations
* Delete from kruize_recommendations only if delete from kruize_experiments succeeds.
*
* @param experimentName
* @return
*/
@Override
public ValidationOutputData deleteKruizeLMExperimentEntryByName(String experimentName) {
ValidationOutputData validationOutputData = new ValidationOutputData(false, null, null);
Transaction tx = null;
try (Session session = KruizeHibernateUtil.getSessionFactory().openSession()) {
try {
tx = session.beginTransaction();
Query query = session.createQuery(DELETE_FROM_LM_EXPERIMENTS_BY_EXP_NAME, null);
query.setParameter("experimentName", experimentName);
int deletedCount = query.executeUpdate();
if (deletedCount == 0) {
validationOutputData.setSuccess(false);
validationOutputData.setMessage("KruizeLMExperimentEntry not found with experiment name: " + experimentName);
} else {
// Remove the experiment from the Recommendations table
Query kruizeLMRecommendationEntryquery = session.createQuery(DELETE_FROM_LM_RECOMMENDATIONS_BY_EXP_NAME, null);
kruizeLMRecommendationEntryquery.setParameter("experimentName", experimentName);
kruizeLMRecommendationEntryquery.executeUpdate();
validationOutputData.setSuccess(true);
}
tx.commit();
} catch (HibernateException e) {
LOGGER.error("Not able to delete experiment {} due to {}", experimentName, e.getMessage());
if (tx != null) tx.rollback();
e.printStackTrace();
validationOutputData.setSuccess(false);
validationOutputData.setMessage(e.getMessage());
//todo save error to API_ERROR_LOG
}
} catch (Exception e) {
LOGGER.error("Not able to delete experiment {} due to {}", experimentName, e.getMessage());
}
return validationOutputData;
}

/**
* Delete metadata with the name dataSourceName
* This deletes the metadata from the KruizeDSMetadataEntry table
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/autotune/database/helper/DBConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ public static final class SQLQUERY {
public static final String SELECT_FROM_METRIC_PROFILE = "from KruizeMetricProfileEntry";
public static final String SELECT_FROM_METRIC_PROFILE_BY_NAME = "from KruizeMetricProfileEntry k WHERE k.name = :name";
public static final String DELETE_FROM_EXPERIMENTS_BY_EXP_NAME = "DELETE FROM KruizeExperimentEntry k WHERE k.experiment_name = :experimentName";
public static final String DELETE_FROM_LM_EXPERIMENTS_BY_EXP_NAME = "DELETE FROM KruizeLMExperimentEntry k WHERE k.experiment_name = :experimentName";
public static final String DELETE_FROM_RESULTS_BY_EXP_NAME = "DELETE FROM KruizeResultsEntry k WHERE k.experiment_name = :experimentName";
public static final String DELETE_FROM_RECOMMENDATIONS_BY_EXP_NAME = "DELETE FROM KruizeRecommendationEntry k WHERE k.experiment_name = :experimentName";
public static final String DELETE_FROM_LM_RECOMMENDATIONS_BY_EXP_NAME = "DELETE FROM KruizeLMRecommendationEntry k WHERE k.experiment_name = :experimentName";
public static final String DELETE_FROM_METADATA_BY_DATASOURCE_NAME = "DELETE FROM KruizeDSMetadataEntry km WHERE km.datasource_name = :dataSourceName";
public static final String DELETE_FROM_METRIC_PROFILE_BY_PROFILE_NAME = "DELETE FROM KruizeMetricProfileEntry km WHERE km.name = :metricProfileName";
public static final String DB_PARTITION_DATERANGE = "CREATE TABLE IF NOT EXISTS %s_%s%s%s PARTITION OF %s FOR VALUES FROM ('%s-%s-%s 00:00:00.000') TO ('%s-%s-%s 23:59:59');";
Expand Down
11 changes: 8 additions & 3 deletions tests/scripts/helpers/kruize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,17 @@ def list_recommendations(experiment_name=None, latest=None, monitoring_end_time=

# Description: This function deletes the experiment and posts the experiment using createExperiment API to Kruize Autotune
# Input Parameters: experiment input json
def delete_experiment(input_json_file, invalid_header=False):
def delete_experiment(input_json_file, invalid_header=False, rm=True):
json_file = open(input_json_file, "r")
input_json = json.loads(json_file.read())

print("\nDeleting the experiment...")
url = URL + "/createExperiment"
if rm:
Copy link
Member

Choose a reason for hiding this comment

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

As we are relaying on the boolean param which is passed to the function to maintain it's string representation, we can directly use the string representation of the boolean instead of placing a if-else block.

I was suggesting something like this:

PARAMS = {'rm': str(rm).lower()}

PARAMS = {'rm': 'true'}
else:
PARAMS = {'rm': 'false'}

print("URL = ", url)

experiment_name = input_json[0]['experiment_name']
Expand All @@ -202,9 +207,9 @@ def delete_experiment(input_json_file, invalid_header=False):
headers = {'content-type': 'application/xml'}
if invalid_header:
print("Invalid header")
response = requests.delete(url, json=delete_json, headers=headers)
response = requests.delete(url, json=delete_json, headers=headers, params=PARAMS)
else:
response = requests.delete(url, json=delete_json)
response = requests.delete(url, json=delete_json, params=PARAMS)

print(response)
print("Response status code = ", response.status_code)
Expand Down
12 changes: 6 additions & 6 deletions tests/scripts/local_monitoring_tests/rest_apis/test_create_experiment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_create_exp_valid_tests(test_name, expected_status_code, version, experi
input_json_file = tmp_json_file
form_kruize_url(cluster_type)

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)

# Create experiment using the specified json
Expand All @@ -108,7 +108,7 @@ def test_create_exp_valid_tests(test_name, expected_status_code, version, experi
assert data['status'] == SUCCESS_STATUS
assert data['message'] == CREATE_EXP_SUCCESS_MSG

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)


Expand Down Expand Up @@ -189,7 +189,7 @@ def test_create_exp_invalid_tests(test_name, expected_status_code, expected_erro
input_json_file = tmp_json_file
form_kruize_url(cluster_type)

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)

# Create experiment using the specified json
Expand All @@ -202,7 +202,7 @@ def test_create_exp_invalid_tests(test_name, expected_status_code, expected_erro
assert data['status'] == ERROR_STATUS
assert data['message'] == expected_error_msg

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)


Expand All @@ -215,7 +215,7 @@ def test_create_multiple_namespace_exp(cluster_type):
input_json_file = "../json_files/create_multiple_namespace_exp.json"
form_kruize_url(cluster_type)

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)

# Create experiment using the specified json
Expand All @@ -229,5 +229,5 @@ def test_create_multiple_namespace_exp(cluster_type):
# validate error message
assert data['message'] == CREATE_EXP_BULK_ERROR_MSG

response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)
9 changes: 5 additions & 4 deletions tests/scripts/local_monitoring_tests/rest_apis/test_list_recommendations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import pytest
import sys

sys.path.append("../../")

from helpers.all_terms_list_reco_json_schema import all_terms_list_reco_json_schema
Expand Down Expand Up @@ -100,7 +101,7 @@ def test_list_recommendations_namespace_single_result(test_name, expected_status
input_json_file = tmp_json_file

form_kruize_url(cluster_type)
response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)

#Install default metric profile
Expand Down Expand Up @@ -169,7 +170,7 @@ def test_list_recommendations_namespace_single_result(test_name, expected_status
validate_local_monitoring_reco_json(namespace_exp_json[0], list_reco_json[0])

# Delete experiment
response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

Expand Down Expand Up @@ -249,7 +250,7 @@ def test_accelerator_recommendation_if_exists(
input_json_file = tmp_json_file

form_kruize_url(cluster_type)
response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)

#Install default metric profile
Expand Down Expand Up @@ -316,6 +317,6 @@ def test_accelerator_recommendation_if_exists(
validate_accelerator_recommendations_for_container(list_reco_json)

# Delete experiment
response = delete_experiment(input_json_file)
response = delete_experiment(input_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE
15 changes: 7 additions & 8 deletions tests/scripts/local_monitoring_tests/rest_apis/test_local_monitoring_e2e_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,16 @@ def test_list_recommendations_multiple_exps_for_datasource_workloads(cluster_typ
tfb_exp_json_file = "../json_files/create_tfb_exp.json"
tfb_db_exp_json_file = "../json_files/create_tfb_db_exp.json"


response = delete_experiment(tfb_exp_json_file)
response = delete_experiment(tfb_exp_json_file, rm=False)
print("delete tfb exp = ", response.status_code)

response = delete_experiment(tfb_db_exp_json_file)
response = delete_experiment(tfb_db_exp_json_file, rm=False)
print("delete tfb_db exp = ", response.status_code)

response = delete_experiment(namespace_exp_json_file)
response = delete_experiment(namespace_exp_json_file, rm=False)
print("delete namespace exp = ", response.status_code)

#Install default metric profile
# Install default metric profile
if cluster_type == "minikube":
metric_profile_json_file = metric_profile_dir / 'resource_optimization_local_monitoring_norecordingrules.json'

Expand Down Expand Up @@ -292,17 +291,17 @@ def test_list_recommendations_multiple_exps_for_datasource_workloads(cluster_typ
validate_local_monitoring_reco_json(namespace_exp_json[0], list_reco_json[0])

# Delete tfb experiment
response = delete_experiment(tfb_exp_json_file)
response = delete_experiment(tfb_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

# Delete tfb_db experiment
response = delete_experiment(tfb_db_exp_json_file)
response = delete_experiment(tfb_db_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

# Delete namespace experiment
response = delete_experiment(namespace_exp_json_file)
response = delete_experiment(namespace_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

Expand Down
12 changes: 6 additions & 6 deletions tests/scripts/local_monitoring_tests/rest_apis/test_namespace_reco_e2e_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ def test_list_recommendations_namespace_exps(cluster_type):
ns3_exp_json_file = tmp_json_file_3

# delete tfb experiments
response = delete_experiment(ns1_exp_json_file)
response = delete_experiment(ns1_exp_json_file, rm=False)
print("delete tfb exp = ", response.status_code)

response = delete_experiment(ns2_exp_json_file)
response = delete_experiment(ns2_exp_json_file, rm=False)
print("delete tfb_db exp = ", response.status_code)

response = delete_experiment(ns3_exp_json_file)
response = delete_experiment(ns3_exp_json_file, rm=False)
print("delete namespace exp = ", response.status_code)

#Install default metric profile
Expand Down Expand Up @@ -326,15 +326,15 @@ def test_list_recommendations_namespace_exps(cluster_type):


# Delete namespace experiment
response = delete_experiment(ns1_exp_json_file)
response = delete_experiment(ns1_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

response = delete_experiment(ns2_exp_json_file)
response = delete_experiment(ns2_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

response = delete_experiment(ns3_exp_json_file)
response = delete_experiment(ns3_exp_json_file, rm=False)
print("delete exp = ", response.status_code)
assert response.status_code == SUCCESS_STATUS_CODE

Expand Down
Loading