[CLIPPER-57] Read existing configuration from Redis on startup#123

Merged

dcrankshaw merged 7 commits intoucbrise:developfrom

giulio-zhou:read-config-on-start

Apr 21, 2017

Contributor

giulio-zhou commented Apr 17, 2017

Create two functions get_all_model_names() and
get_all_application_names() which call the KEYS command with a
wildcard (*) argument on the model and application table
Update query frontend to (1) add existing applications and
(2) fill internal state with current model name and versions
on startup

giulio-zhou added 2 commits

April 17, 2017 15:37


          [CLIPPER-57] Read existing configuration from Redis on startup

ea5a546

* Create two functions get_all_model_names() and
  get_all_application_names() which call the KEYS command with a
  wildcard (*) argument on the model and application table
* Update query frontend to (1) add existing applications and
  (2) fill internal state with current model name and versions
  on startup


          clang-format query_frontend.hpp and redis_test.cpp

144c6cb

giulio-zhou requested a review from dcrankshaw

April 17, 2017 22:58

AmplabJenkins commented Apr 17, 2017

Can one of the admins verify this patch?

Contributor

dcrankshaw commented Apr 19, 2017

jenkins test this please

AmplabJenkins commented Apr 19, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/159/
Test FAILed.

dcrankshaw requested changes

View reviewed changes

Contributor

dcrankshaw left a comment

This looks good. I tested it locally and it seems to work. We should add unit tests for this though.

Add some tests to the query_frontend that first populate the application/model tables using Redis then create a new query frontend RequestHandler object and make sure it reads Redis correctly and adds the right models/versions and applications.

Note that when adding models you'll need explicitly set the current model version as well because the management frontend does that (https://github.com/ucbrise/clipper/blob/develop/src/management/src/management_frontend.hpp#L282-L314).

Make sure to have separate tests for models and applications.

src/frontends/src/query_frontend.hpp Outdated

+                  // (1) Iterate through applications and set up predict/update endpoints.
+                  for (std::string app_name :
+                       clipper::redis::get_all_application_names(redis_connection_)) {
+                    // Is there a way/need to not repeat these function calls? (from above)

Contributor

dcrankshaw Apr 19, 2017

Repeating them is fine. There's not much duplicate code.

src/frontends/src/query_frontend.hpp

+                    std::vector<std::string> candidate_model_names =
+                        clipper::redis::str_to_model_names(app_info["candidate_model_names"]);
+                    InputType input_type = clipper::parse_input_type(app_info["input_type"]);
+                    std::string policy = app_info["policy"];

Contributor

dcrankshaw Apr 19, 2017

This PR is based on an old commit. Update to the latest version. You'll notice that applications no longer have a policy field and now have a default_output field that needs to be set.

Contributor Author

giulio-zhou Apr 20, 2017

Updated to include default_output.

src/frontends/src/query_frontend.hpp Outdated

+                       clipper::redis::get_all_model_names(redis_connection_)) {
+                    auto model_version = clipper::redis::get_current_model_version(
+                        redis_connection_, model_name);
+                    // Error handle model_version being < 0?

Contributor

dcrankshaw Apr 19, 2017

Yeah you should check for it. If it is < 0, log an error then throw a runtime exception.

Contributor Author

giulio-zhou Apr 20, 2017

Checked in new implementation with corresponding test.

src/frontends/src/query_frontend.hpp Outdated

+                  // Read from Redis configuration tables and update models/applications
+                  // (1) Iterate through applications and set up predict/update endpoints.
+                  for (std::string app_name :
+                       clipper::redis::get_all_application_names(redis_connection_)) {

Contributor

dcrankshaw Apr 19, 2017

Can you save the vector of existing app names and log them in a useful message. Something like "Found 3 existing applications registered in Clipper: app_name_1, app_name_2, app_name_3."

Contributor Author

giulio-zhou Apr 20, 2017

Yeah, will do.

src/frontends/src/query_frontend.hpp

+                    // Error handle model_version being < 0?
+                    std::unique_lock<std::mutex> l(current_model_versions_mutex_);
+                    current_model_versions_[model_name] = model_version;
+                  }

Contributor

dcrankshaw Apr 19, 2017

After you've initialized the current_model_versions_ map in this for loop, can you log the results similar to the existing applications log message: "Found 3 models deployed to Clipper: model_a@v4, model_b@v7, model_c@v1"

Contributor Author

giulio-zhou Apr 20, 2017

Done.

dcrankshaw mentioned this pull request

[CLIPPER-117] Application utilities #125

Merged

giulio-zhou added 3 commits

April 19, 2017 18:33


          Merge branch 'develop' into read-config-on-start

5d32199


          Test query_frontend initialization and log useful app/model info.

69f3e7d


          Update query_frontend_test and comment

aa83a1d

Contributor

dcrankshaw commented Apr 21, 2017

jenkins test this please

AmplabJenkins commented Apr 21, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/168/
Test FAILed.

dcrankshaw added 2 commits

April 21, 2017 13:15


          Merge and dedupe get_all_applications function

5548a11


          flush redis between unit-tests

7dc933c

Contributor

dcrankshaw commented Apr 21, 2017

jenkins test this please

dcrankshaw approved these changes

View reviewed changes

AmplabJenkins commented Apr 21, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/171/
Test PASSed.

dcrankshaw merged commit 668bf05 into ucbrise:develop

withsmilo added a commit to withsmilo/clipper that referenced this pull request


          Upgrade SimpleWebServer to version 2.1.1

e0b64bb

Currently Clipper uses SimpleWebServer([Oct 25, 2016](https://github.com/eidheim/Simple-Web-Server/tree/8e0d3142bfd3c4932d9c7a9b9fcd517b7a4ec05b)) for http serving.
We need to upgrade this module to resolve some bugs and support keep-alive connection.
So we applied all patches except of below to reach [version 2.1.1 (May 27, 2017)](https://github.com/eidheim/Simple-Web-Server/blob/v2.1.1/server_http.hpp).
Diff between SimpleWebServer v2.1.1(left). and SimpleWebServer(right) in this PR is [here](https://www.diffchecker.com/PI1eJFwo).

* applied patch list
  * Nov 7, 2016: [Fixes ucbrise#82: wrong reset method called in ::start](eidheim/Simple-Web-Server@8da3ad4)
  * Nov 10, 2016: [Fixed crash if server instance gets deleted after the call to io_serv](eidheim/Simple-Web-Server@743785b)
  * Nov 23, 2016: [Fixes ucbrise#86: can now set timeout on client requests](eidheim/Simple-Web-Server@8a73cb3)
  * Nov 23, 2016: [Minor timeout source cleanups](eidheim/Simple-Web-Server@7d95360)
  * Nov 23: 2016: [Simplified Server::parse_request](eidheim/Simple-Web-Server@0d8052d)
  * Dec 4, 2016: [string::substr comparisons replaced by string::compare](eidheim/Simple-Web-Server@14d848b)
  * Dec 13, 2016: [compile clean with gcc 4.6.3](eidheim/Simple-Web-Server@dc74f77)
  * Dec 19, 2016: [Added client verification when a verify file is passed to Server<HTTP](eidheim/Simple-Web-Server@7a97f82)
  * Dec 19, 2016: [Added error reporting through on_error std::function](eidheim/Simple-Web-Server@eef8a10)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@d19244e)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@db95a64)
  * Dec 29, 2016: [Cleanup of server-constructors. Previous constructors have been marked](eidheim/Simple-Web-Server@6c3a59d)
  * Dec 29, 2016: [Bugfix for last commit: config.timeout_content now correctly set in old constructor](eidheim/Simple-Web-Server@175d4dd#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 30, 2016: [Fixed DEPRECATED macro in cases where it is already defined](eidheim/Simple-Web-Server@8c8ef39#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 31, 2016: [Case insensitive header cleanup. Also cleanup and additions to parse_test](eidheim/Simple-Web-Server@549bc64#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 1, 2017: [Added on_upgrade for cases where one wants to handle connection upgrades](eidheim/Simple-Web-Server@bfcb325#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 11, 2017: [Fixed Boost.Regex workaround in regex_orderable. Fixes ucbrise#100](eidheim/Simple-Web-Server@19627bb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Possible implementation for fixing ucbrise#106](eidheim/Simple-Web-Server@600fbe3#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Renamed close_connection_after_send to close_connection_after_response](eidheim/Simple-Web-Server@de560e8#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Added query string parsing and member to request](eidheim/Simple-Web-Server@a4dd2e6#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Travis CI build failed](eidheim/Simple-Web-Server@d554c13#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 14, 2017: [remove locale dependent stof()](eidheim/Simple-Web-Server@50ce751#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 27, 2017: [If query string is present then cut it from the reqeust path so find](eidheim/Simple-Web-Server@e585a7a#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 6, 2017: [Added support for request header Connection: keep-alive (see ucbrise#123)](eidheim/Simple-Web-Server@cfafbcb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Finished query string parsing implementation (PR ucbrise#109)](eidheim/Simple-Web-Server@550bbfe#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Fixed g++ error in Server::Request::parse_query_string](eidheim/Simple-Web-Server@4469de1#diff-be05a8666afdaf0bcadd9499d09cf33c)

* excluded patch list
  * Jan 2, 2017: [Code simplification: got rid of opt_resource at minimal cost](eidheim/Simple-Web-Server@8cdebfb)
  * Jan 2, 2017: [Added warning to Server::resource](eidheim/Simple-Web-Server@fa8c381)
  * Jan 3, 2017: [Removed unnecessary public](eidheim/Simple-Web-Server@f5e65bf)

withsmilo added a commit to withsmilo/clipper that referenced this pull request


          Upgrade SimpleWebServer to version 2.1.1

e7c8830

Currently Clipper uses SimpleWebServer([Oct 25, 2016](https://github.com/eidheim/Simple-Web-Server/tree/8e0d3142bfd3c4932d9c7a9b9fcd517b7a4ec05b)) for http serving.
We need to upgrade this module to resolve some bugs and support keep-alive connection.
So we applied all patches except of some to reach [version 2.1.1 (May 27, 2017)](https://github.com/eidheim/Simple-Web-Server/blob/v2.1.1/server_http.hpp).
Diff between SimpleWebServer v2.1.1(left). and SimpleWebServer(right) in this PR is [here](https://www.diffchecker.com/PI1eJFwo).

* applied patch list
  * Nov 7, 2016: [Fixes ucbrise#82: wrong reset method called in ::start](eidheim/Simple-Web-Server@8da3ad4)
  * Nov 10, 2016: [Fixed crash if server instance gets deleted after the call to io_serv](eidheim/Simple-Web-Server@743785b)
  * Nov 23, 2016: [Fixes ucbrise#86: can now set timeout on client requests](eidheim/Simple-Web-Server@8a73cb3)
  * Nov 23, 2016: [Minor timeout source cleanups](eidheim/Simple-Web-Server@7d95360)
  * Nov 23: 2016: [Simplified Server::parse_request](eidheim/Simple-Web-Server@0d8052d)
  * Dec 4, 2016: [string::substr comparisons replaced by string::compare](eidheim/Simple-Web-Server@14d848b)
  * Dec 13, 2016: [compile clean with gcc 4.6.3](eidheim/Simple-Web-Server@dc74f77)
  * Dec 19, 2016: [Added client verification when a verify file is passed to Server<HTTP](eidheim/Simple-Web-Server@7a97f82)
  * Dec 19, 2016: [Added error reporting through on_error std::function](eidheim/Simple-Web-Server@eef8a10)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@d19244e)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@db95a64)
  * Dec 29, 2016: [Cleanup of server-constructors. Previous constructors have been marked](eidheim/Simple-Web-Server@6c3a59d)
  * Dec 29, 2016: [Bugfix for last commit: config.timeout_content now correctly set in old constructor](eidheim/Simple-Web-Server@175d4dd#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 30, 2016: [Fixed DEPRECATED macro in cases where it is already defined](eidheim/Simple-Web-Server@8c8ef39#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 31, 2016: [Case insensitive header cleanup. Also cleanup and additions to parse_test](eidheim/Simple-Web-Server@549bc64#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 1, 2017: [Added on_upgrade for cases where one wants to handle connection upgrades](eidheim/Simple-Web-Server@bfcb325#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 11, 2017: [Fixed Boost.Regex workaround in regex_orderable. Fixes ucbrise#100](eidheim/Simple-Web-Server@19627bb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Possible implementation for fixing ucbrise#106](eidheim/Simple-Web-Server@600fbe3#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Renamed close_connection_after_send to close_connection_after_response](eidheim/Simple-Web-Server@de560e8#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Added query string parsing and member to request](eidheim/Simple-Web-Server@a4dd2e6#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Travis CI build failed](eidheim/Simple-Web-Server@d554c13#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 14, 2017: [remove locale dependent stof()](eidheim/Simple-Web-Server@50ce751#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 27, 2017: [If query string is present then cut it from the reqeust path so find](eidheim/Simple-Web-Server@e585a7a#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 6, 2017: [Added support for request header Connection: keep-alive (see ucbrise#123)](eidheim/Simple-Web-Server@cfafbcb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Finished query string parsing implementation (PR ucbrise#109)](eidheim/Simple-Web-Server@550bbfe#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Fixed g++ error in Server::Request::parse_query_string](eidheim/Simple-Web-Server@4469de1#diff-be05a8666afdaf0bcadd9499d09cf33c)

* excluded patch list
  * Jan 2, 2017: [Code simplification: got rid of opt_resource at minimal cost](eidheim/Simple-Web-Server@8cdebfb)
  * Jan 2, 2017: [Added warning to Server::resource](eidheim/Simple-Web-Server@fa8c381)
  * Jan 3, 2017: [Removed unnecessary public](eidheim/Simple-Web-Server@f5e65bf)

dcrankshaw pushed a commit that referenced this pull request


          Upgrade SimpleWebServer to version 2.1.1 (#438)

1ed8f3a

* Upgrade SimpleWebServer to version 2.1.1

Currently Clipper uses SimpleWebServer([Oct 25, 2016](https://github.com/eidheim/Simple-Web-Server/tree/8e0d3142bfd3c4932d9c7a9b9fcd517b7a4ec05b)) for http serving.
We need to upgrade this module to resolve some bugs and support keep-alive connection.
So we applied all patches except of some to reach [version 2.1.1 (May 27, 2017)](https://github.com/eidheim/Simple-Web-Server/blob/v2.1.1/server_http.hpp).
Diff between SimpleWebServer v2.1.1(left). and SimpleWebServer(right) in this PR is [here](https://www.diffchecker.com/PI1eJFwo).

* applied patch list
  * Nov 7, 2016: [Fixes #82: wrong reset method called in ::start](eidheim/Simple-Web-Server@8da3ad4)
  * Nov 10, 2016: [Fixed crash if server instance gets deleted after the call to io_serv](eidheim/Simple-Web-Server@743785b)
  * Nov 23, 2016: [Fixes #86: can now set timeout on client requests](eidheim/Simple-Web-Server@8a73cb3)
  * Nov 23, 2016: [Minor timeout source cleanups](eidheim/Simple-Web-Server@7d95360)
  * Nov 23: 2016: [Simplified Server::parse_request](eidheim/Simple-Web-Server@0d8052d)
  * Dec 4, 2016: [string::substr comparisons replaced by string::compare](eidheim/Simple-Web-Server@14d848b)
  * Dec 13, 2016: [compile clean with gcc 4.6.3](eidheim/Simple-Web-Server@dc74f77)
  * Dec 19, 2016: [Added client verification when a verify file is passed to Server<HTTP](eidheim/Simple-Web-Server@7a97f82)
  * Dec 19, 2016: [Added error reporting through on_error std::function](eidheim/Simple-Web-Server@eef8a10)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@d19244e)
  * Dec 19, 2016: [Minor cleanup](eidheim/Simple-Web-Server@db95a64)
  * Dec 29, 2016: [Cleanup of server-constructors. Previous constructors have been marked](eidheim/Simple-Web-Server@6c3a59d)
  * Dec 29, 2016: [Bugfix for last commit: config.timeout_content now correctly set in old constructor](eidheim/Simple-Web-Server@175d4dd#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 30, 2016: [Fixed DEPRECATED macro in cases where it is already defined](eidheim/Simple-Web-Server@8c8ef39#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Dec 31, 2016: [Case insensitive header cleanup. Also cleanup and additions to parse_test](eidheim/Simple-Web-Server@549bc64#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 1, 2017: [Added on_upgrade for cases where one wants to handle connection upgrades](eidheim/Simple-Web-Server@bfcb325#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 11, 2017: [Fixed Boost.Regex workaround in regex_orderable. Fixes #100](eidheim/Simple-Web-Server@19627bb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Possible implementation for fixing #106](eidheim/Simple-Web-Server@600fbe3#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Jan 24, 2017: [Renamed close_connection_after_send to close_connection_after_response](eidheim/Simple-Web-Server@de560e8#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Added query string parsing and member to request](eidheim/Simple-Web-Server@a4dd2e6#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 5, 2017: [Travis CI build failed](eidheim/Simple-Web-Server@d554c13#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 14, 2017: [remove locale dependent stof()](eidheim/Simple-Web-Server@50ce751#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * Feb 27, 2017: [If query string is present then cut it from the reqeust path so find](eidheim/Simple-Web-Server@e585a7a#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 6, 2017: [Added support for request header Connection: keep-alive (see #123)](eidheim/Simple-Web-Server@cfafbcb#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Finished query string parsing implementation (PR #109)](eidheim/Simple-Web-Server@550bbfe#diff-be05a8666afdaf0bcadd9499d09cf33c)
  * May 27, 2017: [Fixed g++ error in Server::Request::parse_query_string](eidheim/Simple-Web-Server@4469de1#diff-be05a8666afdaf0bcadd9499d09cf33c)

* excluded patch list
  * Jan 2, 2017: [Code simplification: got rid of opt_resource at minimal cost](eidheim/Simple-Web-Server@8cdebfb)
  * Jan 2, 2017: [Added warning to Server::resource](eidheim/Simple-Web-Server@fa8c381)
  * Jan 3, 2017: [Removed unnecessary public](eidheim/Simple-Web-Server@f5e65bf)

* Run format_code.sh

* format code

* Run run_unittests.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet