Skip to content

Commit

Permalink
Implement delete_model_links API
Browse files Browse the repository at this point in the history
* related issue : ucbrise#603
* Delete the link between a model and an app.

> Description
> -----------
> unlink_model_from_app(app_name, model_name)

> Parameters
> ----------
> app_name : str
>     The name of the application
> model_name : str
>     The name of the model to link to the application

> Raises
> ------
> :py:exc:`clipper.UnconnectedException`
>     versions. All replicas for each version of each model will be stopped.

> API path
> --------
> POST /admin/delete_model_links

> Request schema
> --------------
> const std::string DELETE_MODEL_LINKS_JSON_SCHEMA = R"(
>   {
>   "app_name" := string,
>   "model_names" := [string]
>   }
> )";
  • Loading branch information
Sungjun.Kim committed Mar 22, 2019
1 parent 847d6de commit d008c04
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 1 deletion.
43 changes: 42 additions & 1 deletion clipper_admin/clipper_admin/clipper_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,47 @@ def link_model_to_app(self, app_name, model_name):
"Model {model} is now linked to application {app}".format(
model=model_name, app=app_name))

def unlink_model_from_app(self, app_name, model_name):
"""
Prevents the model with `model_name` from being used by the app with `app_name`.
The model and app should both be registered with Clipper and a link should
already exist between them.
Parameters
----------
app_name : str
The name of the application
model_name : str
The name of the model to link to the application
Raises
------
:py:exc:`clipper.UnconnectedException`
:py:exc:`clipper.ClipperException`
"""

if not self.connected:
raise UnconnectedException()

url = "http://{host}/admin/delete_model_links".format(
host=self.cm.get_admin_addr())
req_json = json.dumps({
"app_name": app_name,
"model_names": [model_name]
})
headers = {'Content-type': 'application/json'}
r = requests.post(url, headers=headers, data=req_json)
logger.debug(r.text)
if r.status_code != requests.codes.ok:
msg = "Received error status code: {code} and message: {msg}".format(
code=r.status_code, msg=r.text)
logger.error(msg)
raise ClipperException(msg)
else:
logger.info(
"Model {model} is now removed to application {app}".format(
model=model_name, app=app_name))

def build_and_deploy_model(self,
name,
version,
Expand Down Expand Up @@ -859,7 +900,7 @@ def get_linked_models(self, app_name):
-------
list
Returns a list of the names of models linked to the app.
If no models are linked to the specified app, None is returned.
If no models are linked to the specified app, empty list is returned.
Raises
------
Expand Down
19 changes: 19 additions & 0 deletions integration-tests/clipper_admin_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,25 @@ def test_link_registered_model_to_app_succeeds(self):
result = self.clipper_conn.get_linked_models(app_name)
self.assertEqual([model_name], result)

def test_unlink_registered_model_from_app_succeeds(self):
# Register app
app_name = "testapp"
input_type = "doubles"
default_output = "DEFAULT"
slo_micros = 30000
self.clipper_conn.register_application(app_name, input_type,
default_output, slo_micros)

# Register model
model_name = "m"
self.clipper_conn.register_model(model_name, "v1", input_type)

self.clipper_conn.link_model_to_app(app_name, model_name)
self.clipper_conn.unlink_model_from_app(app_name, model_name)

result = self.clipper_conn.get_linked_models(app_name)
self.assertEqual([], result)

def get_app_info_for_registered_app_returns_info_dictionary(self):
# Register app
app_name = "testapp"
Expand Down
7 changes: 7 additions & 0 deletions src/frontends/src/query_frontend.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,13 @@ class RequestHandler {
auto linked_model_names =
clipper::redis::get_linked_models(redis_connection_, app_name);
set_linked_models_for_app(app_name, linked_model_names);

} else if (event_type == "srem") {
clipper::log_info_formatted(LOGGING_TAG_QUERY_FRONTEND,
"Model link removal detected for app: {}", app_name);
auto linked_model_names =
clipper::redis::get_linked_models(redis_connection_, app_name);
set_linked_models_for_app(app_name, linked_model_names);
}
});

Expand Down
8 changes: 8 additions & 0 deletions src/libclipper/include/clipper/redis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,14 @@ bool add_application(redox::Redox& redis, const std::string& app_name,
bool add_model_links(redox::Redox& redis, const std::string& app_name,
const std::vector<std::string>& model_names);

/**
* Deletes links between the specified app and models.
*
* \return Returns true if the removal was successful.
*/
bool delete_model_links(redox::Redox& redis, const std::string& app_name,
const std::vector<std::string>& model_names);

/**
* Deletes a container from the container table if it exists.
*
Expand Down
16 changes: 16 additions & 0 deletions src/libclipper/src/redis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,22 @@ bool add_model_links(redox::Redox& redis, const std::string& appname,
}
}

bool delete_model_links(redox::Redox& redis, const std::string& appname,
const std::vector<std::string>& model_names) {
if (send_cmd_no_reply<string>(
redis, {"SELECT", std::to_string(REDIS_APP_MODEL_LINKS_DB_NUM)})) {
for (auto model_name : model_names) {
if (!send_cmd_no_reply<int>(
redis, vector<string>{"SREM", appname, model_name})) {
return false;
}
}
return true;
} else {
return false;
}
}

bool delete_application(redox::Redox& redis, const std::string& appname) {
if (send_cmd_no_reply<string>(
redis, {"SELECT", std::to_string(REDIS_APPLICATION_DB_NUM)})) {
Expand Down
31 changes: 31 additions & 0 deletions src/libclipper/test/redis_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,37 @@ TEST_F(RedisTest, AddModelLinks) {
ASSERT_EQ(model_names, linked_models);
}

TEST_F(RedisTest, DeleteModelLinks) {
std::string app_name = "my_app_name";
InputType input_type = InputType::Doubles;
std::string policy = DefaultOutputSelectionPolicy::get_name();
std::string default_output = "1.0";
int latency_slo_micros = 10000;
ASSERT_TRUE(add_application(*redis_, app_name, input_type, policy,
default_output, latency_slo_micros));

std::vector<std::string> labels{"ads", "images", "experimental", "other",
"labels"};
std::string model_name_1 = "model_1";
std::string model_name_2 = "model_2";
VersionedModelId model_1 = VersionedModelId(model_name_1, "1");
VersionedModelId model_2 = VersionedModelId(model_name_2, "1");
std::string container_name = "clipper/test_container";
std::string model_path = "/tmp/models/m/1";
ASSERT_TRUE(add_model(*redis_, model_1, input_type, labels, container_name,
model_path, DEFAULT_BATCH_SIZE));
ASSERT_TRUE(add_model(*redis_, model_2, input_type, labels, container_name,
model_path, DEFAULT_BATCH_SIZE));

std::vector<std::string> model_names =
std::vector<std::string>{model_name_1, model_name_2};
ASSERT_TRUE(add_model_links(*redis_, app_name, model_names));
ASSERT_TRUE(delete_model_links(*redis_, app_name, model_names));

auto linked_models = get_linked_models(*redis_, app_name);
ASSERT_EQ(versions.size(), (size_t)0);
}

TEST_F(RedisTest, SetCurrentModelVersion) {
std::string model_name = "mymodel";
ASSERT_TRUE(set_current_model_version(*redis_, model_name, "2"));
Expand Down
81 changes: 81 additions & 0 deletions src/management/src/management_frontend.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const std::string GET_ALL_MODELS = ADMIN_PATH + "/get_all_models$";
const std::string GET_MODEL = ADMIN_PATH + "/get_model$";
const std::string GET_ALL_CONTAINERS = ADMIN_PATH + "/get_all_containers$";
const std::string GET_CONTAINER = ADMIN_PATH + "/get_container$";
const std::string DELETE_MODEL_LINKS = ADMIN_PATH + "/delete_model_links$";

const std::string PING = ADMIN_PATH + "/ping$";

Expand All @@ -84,6 +85,13 @@ const std::string ADD_MODEL_LINKS_JSON_SCHEMA = R"(
}
)";

const std::string DELETE_MODEL_LINKS_JSON_SCHEMA = R"(
{
"app_name" := string,
"model_names" := [string]
}
)";

const std::string GET_LINKED_MODELS_REQUESTS_SCHEMA = R"(
{
"app_name" := string
Expand Down Expand Up @@ -241,6 +249,27 @@ class RequestHandler {
respond_http(e.what(), "400 Bad Request", response);
}
});
server_.add_endpoint(
DELETE_MODEL_LINKS, "POST",
[this](std::shared_ptr<HttpServer::Response> response,
std::shared_ptr<HttpServer::Request> request) {
try {
clipper::log_info(LOGGING_TAG_MANAGEMENT_FRONTEND,
"Remove application links POST request");
std::string result = delete_model_links(request->content.string());
respond_http(result, "200 OK", response);
} catch (const json_parse_error& e) {
std::string err_msg =
json_error_msg(e.what(), DELETE_MODEL_LINKS_JSON_SCHEMA);
respond_http(err_msg, "400 Bad Request", response);
} catch (const json_semantic_error& e) {
std::string err_msg =
json_error_msg(e.what(), DELETE_MODEL_LINKS_JSON_SCHEMA);
respond_http(err_msg, "400 Bad Request", response);
} catch (const std::invalid_argument& e) {
respond_http(e.what(), "400 Bad Request", response);
}
});
server_.add_endpoint(
ADD_MODEL, "POST",
[this](std::shared_ptr<HttpServer::Response> response,
Expand Down Expand Up @@ -620,6 +649,58 @@ class RequestHandler {
}
}

/**
* Creates an endpoint that listens for requests to remove links between
* apps and models
*
* JSON format:
* {
* "app_name" := string,
* "model_names" := [string]
* }
*/
std::string delete_model_links(const std::string& json) {
rapidjson::Document d;
parse_json(json, d);

std::string app_name = get_string(d, "app_name");
std::vector<string> model_names = get_string_array(d, "model_names");

// Confirm that the app exists
auto app_info =
clipper::redis::get_application(redis_connection_, app_name);
if (app_info.size() == 0) {
std::stringstream ss;
ss << "No app with name " << app_name << " exists.";
throw std::invalid_argument(ss.str());
}

// Confirm that the model names supplied are of linked models
auto existing_linked_models =
clipper::redis::get_linked_models(redis_connection_, app_name);

for (auto const& model_name : model_names) {
if (std::find(existing_linked_models.begin(),
existing_linked_models.end(),
model_name) == existing_linked_models.end()) {
std::stringstream ss;
ss << "Cannot remove nonexistent link between app " << app_name
<< " and model " << model_name;
throw std::invalid_argument(ss.str());
}
}

if (clipper::redis::delete_model_links(redis_connection_, app_name,
model_names)) {
return "Success!";
} else {
std::stringstream ss;
ss << "Error deleting linked models from " << app_name << " in Redis";
throw std::invalid_argument(ss.str());
}
}


/**
* Processes a request to add a new application to Clipper
*
Expand Down

0 comments on commit d008c04

Please sign in to comment.