PerfEnforce on Myria #868

jortiz16 · 2017-01-16T18:45:16Z

This PR adds the PerfEnforce research component to Myria.

I tried to keep PerfEnforce as self-contained as possible, but there are a couple features I did have to add to Myria in order to get it to work:

Ability to create materialized views
Ability to execute a SQL command directly on Postgres (uses the DbExecute operator)
Ability to execute a SQL command and return the results in an array
For TupleSink, I added the option to remove the column header

The entry point for the PerfEnforce code is through the PerfEnforceDriver class. This class focuses on two steps:

Ingesting data, collecting statistics and generating the PSLA. This is all enclosed in the PerfEnforceDataPreparation class.
The learning model that will schedule the incoming queries at different cluster sizes in order to meet the SLAs. This can be seen in the PerfEnforceOnlineLearning class.

To actually see this work, the user will need to specify the "PerfEnforce" flag through myria-cluster, which will create a 12-node cluster. The web interface will be sending calls to the PerfEnforce API found in the PerfEnforceResource class.

jingjingwang · 2017-02-09T10:31:27Z

src/edu/washington/escience/myria/api/encoding/PerfEnforceStatisticsEncoding.java

+
+  public PerfEnforceStatisticsEncoding(
+      final String table_name, final long table_size, final List<String> selectivityList) {
+    this(


I think we can just merge these two constructors since the second one is never called except by the first one.

jingjingwang · 2017-02-09T10:36:58Z

src/edu/washington/escience/myria/api/encoding/PerfEnforceTableEncoding.java

+   */
+  @JsonCreator
+  public PerfEnforceTableEncoding(
+      @JsonProperty(value = "relationKey", required = true) final RelationKey relationKey,


Is there a specific reason of using @JsonProperty here but not in PerfEnforceStatisticsEncoding?

Not necessarily. I just didn't use the JSON encoding for PerfEnforceStatisticsEncoding. I will add it to keep it all consistent.

jingjingwang · 2017-02-09T10:38:57Z

src/edu/washington/escience/myria/operator/TupleSink.java

+      final Operator child,
+      final TupleWriter tupleWriter,
+      final DataSink dataSink,
+      boolean includeColumnHeader) {


final boolean

jingjingwang · 2017-02-09T11:14:32Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceUtils.java

+          currentWorkerFeatures =
+              currentWorkerFeatures.substring(currentWorkerFeatures.indexOf("cost"));
+          currentWorkerFeatures = currentWorkerFeatures.replace("\"", " ");
+          String[] cmd = {


This can be replaced by String.replace() to save launching a process.

Thanks! This is better

jingjingwang · 2017-02-09T11:41:30Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceUtils.java

+   * @param keys the set of columns to concatenate
+   * @param schema the schema to create
+   */
+  public static Schema getAttributeKeySchema(final Set<Integer> keys, final Schema schema) {


return schema.getSubSchema(Ints.toArray(keys));

I didn't know this existed! So much nicer now

jingjingwang · 2017-02-09T11:45:57Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceUtils.java

+   * @param keys the set of columns to concatenate
+   * @param schema the schema of the relation
+   */
+  public static String getAttributeKeyString(final Set<Integer> keys, final Schema schema) {


Joiner.on(",").join(getAttributeKeySchema(keys, schema).getColumnNames())

jingjingwang · 2017-02-09T11:48:54Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceUtils.java

+      String explainQuery = "EXPLAIN " + sqlQuery;
+      List<String> featuresList = new ArrayList<String>();
+      String maxFeature = "";
+      double maxCost = Integer.MIN_VALUE;


Just want to check: is Integer.MIN_VALUE small enough? (Is cost always positive?)

I'm pretty sure that the minimum cost for query execution from Postgres is 0.00. I can't find exact document to back this up, but I can't imagine there would be a query cost less than 0. In addition, I'm also only running SPJ queries. I'm not running indexes, stored procedures, or anything outside the ordinary :)

jingjingwang · 2017-02-10T03:48:59Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceOnlineLearning.java

+      if (queryRuntime != 0) {
+        String[] parts = result.split(",");
+        result =
+            parts[0]


Using Joiner would save some space.

jingjingwang · 2017-02-10T03:58:22Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceOnlineLearning.java

+      Process p = Runtime.getRuntime().exec(arrayCommand);
+
+      BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
+      while ((reader.readLine()) != null) {}


Is it intended? If you only want to launch a process I believe there exist ways to discard its output. (There is another place above)

Yes, I ran into an issue where the process would just hang even when I tried to flush the error stream. I ended up following suggestions from http://stackoverflow.com/questions/5483830/process-waitfor-never-returns.

jingjingwang · 2017-02-10T04:02:23Z

src/edu/washington/escience/myria/perfenforce/PerfEnforceOnlineLearning.java

+      while ((currentLine = streamReader.readLine()) != null) {
+        nextQueryPrediction = Double.parseDouble((currentLine.split(",")[0]).split(":")[1]);
+      }
+      streamReader.close();


Put it in finally is safer. (Also there are several places with the similar issue.)

Try-with-resources, not finally :)

Apparently I need to update my knowledge on it :)

jingjingwang · 2017-02-10T04:13:49Z

@jortiz16 I made a pass, although I have to admit that I didn't run any test :) But if it works on your side then I believe it's okay. BTW this branch has conflicts to be resolved.

jingjingwang · 2017-02-10T04:16:41Z

Also just curious: is it going to run in Windows?

jingjingwang · 2017-02-10T06:49:08Z

Another thing is: many Javadocs are missing. If you use Eclipse, I think it checks them automatically if the option is turned on.

coveralls · 2017-02-12T22:01:14Z

Coverage decreased (-0.6%) to 26.497% when pulling 756bb12 on perfenforce-merge into 6c56e3e on master.

jortiz16 · 2017-02-12T22:03:21Z

I had to do a rebase, but the only new commits are the last two ("addressing comments" and "modify based on rebase"). I still need to test to make sure that the new changes did not introduce bugs. For example, I had to change the PerfEnforceDataPreparation because I wasn't using the new BroadcastDistributeFunction.

coveralls · 2017-02-14T03:38:08Z

Coverage decreased (-0.6%) to 26.457% when pulling a70ed21 on perfenforce-merge into d282c44 on master.

jortiz16 · 2017-02-14T10:13:22Z

Ok ready for another pass. I tested these changes on my own cluster deployment. Thanks!

coveralls · 2017-02-14T10:14:58Z

Coverage decreased (-0.6%) to 26.44% when pulling 0eced3c on perfenforce-merge into d282c44 on master.

jortiz16 · 2017-02-20T01:06:23Z

Hey @jingjingwang would you happen to know why the coveralls test fails by any chance? I looked at some of the classes they reference, but it did not seem relevant to the changes I made...

jingjingwang

LGTM, only nitpicking on HTTP methods.

jingjingwang · 2017-03-04T08:11:37Z

src/edu/washington/escience/myria/api/PerfEnforceResource.java

+  public Response prepareData(final List<PerfEnforceTableEncoding> tableList)
+      throws PerfEnforceException, DbException, Exception {
+    server.getPerfEnforceDriver().preparePSLA(tableList);
+    return Response.noContent().build();


I think this maps to HTTP return code 204. I'm not sure if 201 is more appropriate, if this POST creates any resource then 201 sounds better.

jingjingwang · 2017-03-04T08:33:22Z

src/edu/washington/escience/myria/api/PerfEnforceResource.java

+   */
+  @POST
+  @Path("/setTier")
+  @Consumes(MediaType.MULTIPART_FORM_DATA)


Using MULTIPART_FORM_DATA usually means that the input form contains some files, which seems to be not your case. (Same in other places)

jingjingwang · 2017-03-04T08:34:00Z

src/edu/washington/escience/myria/api/PerfEnforceResource.java

+  @Path("/findSLA")
+  @Consumes(MediaType.MULTIPART_FORM_DATA)
+  public Response findSLA(@FormDataParam("querySQL") final String querySQL) throws Exception {
+    server.getPerfEnforceDriver().findSLA(querySQL);


If it's only getting some data but not modifying any server state, then GET is more appropriate.

Maybe I'm misunderstanding, but I have this as a POST since /findSLA requires data to be submitted.

https://github.com/uwescience/myria/blob/master/src/edu/washington/escience/myria/api/QueryResource.java#L238 is an example of using GET with String parameters.

coveralls · 2017-03-06T10:21:14Z

Coverage decreased (-1.4%) to 25.656% when pulling f5984fc on perfenforce-merge into d282c44 on master.

coveralls · 2017-03-06T10:21:19Z

Coverage decreased (-1.4%) to 25.697% when pulling f5984fc on perfenforce-merge into d282c44 on master.

coveralls · 2017-03-07T10:24:10Z

Coverage decreased (-1.4%) to 25.682% when pulling e4d22a8 on perfenforce-merge into d282c44 on master.

jingjingwang · 2017-03-07T21:06:51Z

LGTM!

jortiz16 assigned jingjingwang Jan 16, 2017

jingjingwang reviewed Feb 10, 2017

View reviewed changes

jortiz16 force-pushed the perfenforce-merge branch from 0d07b7b to 756bb12 Compare February 12, 2017 21:58

jortiz16 and others added 20 commits February 13, 2017 19:25

adding perfenforce

3df4398

simplify encoding

129cd66

fixing server parameter and renaming encoding

ad36cc0

formatting

34a621b

fix execute statement

ff10ade

fixing output from execute sql statements

21eea43

fixes for stats

df77046

changing stats to a list output

324cce8

exceptions for debugging and S3 provider chain

a4cf96b

to compile

68518c2

fixes

eec1096

more adjustments and paths

b369515

more path edits and formatting

0c1407f

bug fixes

3e6b30f

adding comments

d7de2a4

formatting

7428f97

only keep Default chain

13f4eb4

addressing comments

13c6a36

modify based on rebase

61f48a4

fixing rebase merge

a70ed21

jortiz16 force-pushed the perfenforce-merge branch from 756bb12 to a70ed21 Compare February 14, 2017 03:34

fix broadcast and round robin calls

0eced3c

jingjingwang reviewed Mar 4, 2017

View reviewed changes

jortiz16 added 2 commits March 6, 2017 02:16

addressing comments on PerfEnforce API

ceddd39

formatting

f5984fc

switching to GET

e4d22a8

jingjingwang closed this Mar 7, 2017

jingjingwang reopened this Mar 7, 2017

jingjingwang merged commit b413944 into master Mar 7, 2017

jingjingwang deleted the perfenforce-merge branch March 7, 2017 21:07

PerfEnforce on Myria #868

PerfEnforce on Myria #868

Conversation

jortiz16 commented Jan 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jingjingwang commented Feb 10, 2017

jingjingwang commented Feb 10, 2017

jingjingwang commented Feb 10, 2017

coveralls commented Feb 12, 2017

jortiz16 commented Feb 12, 2017

coveralls commented Feb 14, 2017

jortiz16 commented Feb 14, 2017

coveralls commented Feb 14, 2017

jortiz16 commented Feb 20, 2017

jingjingwang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 6, 2017

coveralls commented Mar 6, 2017

coveralls commented Mar 7, 2017

jingjingwang commented Mar 7, 2017