Refactor query engine for distributed query support #3299
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -2310,12 +2309,6 @@ func TestServer_Query_LimitAndOffset(t *testing.T) { | |||
| params: url.Values{"db": []string{"db0"}}, | |||
| }, | |||
| &Query{ | |||
| name: "limit + offset equal to the number of points with group by time", | |||
There was a problem hiding this comment.
This test appears to have been duped.
This was referenced Jul 12, 2015
| "github.com/influxdb/influxdb/influxql" | ||
| ) | ||
|
|
||
| // RawMapper is for retrieving data, for a query, from a given shard. It implements the |
There was a problem hiding this comment.
The ShardMapper interface doesn't actually exist, and this comment needs correction.
otoolep
force-pushed
the
dq_squashed
branch
3 times, most recently
from
ae52109
to
aa5a1c6
Compare
|
I just pushed up a change and it failed, looking into it now. |
With this change, the query engine code gathers information about shards and tagsets by working with individual shards, collating the information, and returning that to the client. It does not assume that any particular shard is local, and accesses all shards through abstracted Mappers, of which there are two types -- a Mapper type for Raw queries and a second type for Aggregate queries. There are corresponding Executors for each type of Mapper, but both types of Executors share the same interface.
|
Closing in favour of #3320. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this change, the query engine code gathers information about shards and tagsets by working with individual shards, collating the information, and returning that to the client. However it does not assume that any particular shard is local, and accesses all shards through abstracted Shard Mappers, of which there are two types -- a type for Raw queries and a second type for Aggregate queries. There are corresponding Executors for each type of Mapper, but both types of Executors share the
same interface.
There is also a new Query Planner, which is much simpler than the previous planner. This planner is now part of the
tsdbpackage, and is responsible for determining which shards must be accessed. and accessing those shards either locally, or over the network using the cluster service.Remaining work
The only remaining work is to have the cluster service "wrap" the two new Mappers types, such that remote shards can be mapped as easily as local shards. This should be quite straightforward -- for remote nodes the Co-ordinator node will hit (through the cluster service) a remote endpoint, which will in turn instantiate a suitable Mapper. This Mapper will communicate with the Co-ordinator node over a persistent TCP connection, allowing state to be maintained between the Co-ordinator node and remote node. The new Mappers should require no changes to support this.
The split between Raw and Aggregate queries was put in place because shards are driven significantly differently in each case, and making this separation explicit makes the code easier to develop and understand. However as a result there is some code duplication between the new Mappers and Executors, and further refactoring will take place. This will occur after the distributed query support has been fully completed.
This code is ready for merging as is, and no functional difference should be visible to users of the system. The next patch will then clearly show the changes for network support. Once that change is, distributed query support will be complete, and the current restrictions on replication factor can be removed.
Testing
All pre-existing query tests pass, and some new tests have been added for the Mapeprs. Where changes have been made, it was for reasons such as error message changes. Where the changes to some tests may be questioned, in-line notes have been added.