vmware-archive / quickstep

Quickstep Project
Apache License 2.0
27 stars 13 forks source link

Query Execution State #131

Closed hbdeshmukh closed 8 years ago

hbdeshmukh commented 8 years ago

This PR includes creation of a new class called QueryExecutionState. The functionality of the class is related to book keeping of a query execution which includes:

Earlier, all of the supporting data structures were private members of Foreman. Now they have been pulled out in the class QueryExecutionState. As this class is fairly straightforward, and we had all of its functionality earlier in Foreman, I didn't write test cases for this class.

The next PR will be about QueryManager, which moves all the functions about a query's execution from Foreman to a new class called QueryManager.

Recreating the PR because Travis build wasn't fired in the last PR for some of the commits.

pateljm commented 8 years ago

LTGM. Will merge once Travis comes back green.

zuyu commented 8 years ago

@pateljm When you merge a PR based on squash, we need to do some extras to squash the commit messages.

We need a better commit message than below:

commit f7f3794ef9411338fca928b27aba31ff99fdfd22
Author: Harshad Deshmukh <d.harshad17@gmail.com>
Date:   Wed Apr 6 00:27:52 2016 -0500

    Query Execution State

    * Added QueryExecutionState class

    - A single class that does the book keeping of state of query execution.
    - Encapsulates the various data structures used in Foreman for the
      purpose of this book keeping.

    * Foreman uses query execution state.

    - Replaced various vectors, unordered maps with a single query
      execution state instance in Foreman.
    - Foreman test makes use of query execution state too.

    * Copyright fix.

    * Doxygen fixes for QueryExecutionState.

    * C++11 related and gcc specific fixes.

    * Renaming and minor refactoring in QueryExecutionState.

If I were you, I will do so in two steps:

  1. Update the squashing commit message (use the PR title by default), if any. In this case, Added Query Execution State.
  2. Replace all existing commit messages with the link to the PR. In this case, https://github.com/pivotalsoftware/quickstep/pull/131.

FYI, this is the commit message from the PR I just merged for @pateljm:

commit b37ee538b3891b4a48424f093b42c1ab249ea416
Author: Jignesh Patel <pateljm@users.noreply.github.com>
Date:   Wed Apr 6 00:35:45 2016 -0500

    Drop Catalog Initialization Files.

    https://github.com/pivotalsoftware/quickstep/pull/130

And this is a commit message from Apache Mesos.

commit 87b5159109d3e10323dc8ce505e79dbf6168e6d6
Author: Kevin Klues <klueska@gmail.com>
Date:   Tue Apr 5 17:40:17 2016 -0700

    Added tests for the Nvidia GPU isolator.

    This commit also includes a test filter called 'NvidiaGpuFilter' that
    checks for the existence of 'nvidia-smi' before running any tests that
    contain the 'NVIDIA_GPU_' prefix.

    Review: https://reviews.apache.org/r/44364/