Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for scanning multiple spans for a single index. #2293

Merged
merged 1 commit into from
Aug 28, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion examples/sql_bank/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func moveMoney(db *sql.DB) {
log.Fatal(err)
}
startTime := time.Now()
// Query is very slow and will be fixed by https://github.com/cockroachdb/cockroach/issues/2140
query := fmt.Sprintf("SELECT id, balance FROM accounts WHERE id IN (%d, %d)", from, to)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query is usually no longer very slow, but I still sometimes see log messages like:

I0828 15:07:06.299984 34037 examples/sql_bank/main.go:61 SELECT id, balance FROM accounts WHERE id IN (130, 204) took 3.059484793s

rows, err := tx.Query(query)
elapsed := time.Now().Sub(startTime)
Expand Down
43 changes: 32 additions & 11 deletions sql/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,18 @@ func (q *qvalue) String() string {
type qvalMap map[ColumnID]*qvalue
type colKindMap map[ColumnID]ColumnType_Kind

type span struct {
start proto.Key
end proto.Key
}

// A scanNode handles scanning over the key/value pairs for a table and
// reconstructing them into rows.
type scanNode struct {
txn *client.Txn
desc *TableDescriptor
index *IndexDescriptor
startKey proto.Key
endKey proto.Key
spans []span
visibleCols []ColumnDescriptor
isSecondaryIndex bool
reverse bool
Expand Down Expand Up @@ -231,22 +235,39 @@ func (n *scanNode) initScan() bool {
return true
}

// Retrieve all of the keys that start with our index key prefix.
if len(n.startKey) == 0 {
n.startKey = proto.Key(MakeIndexKeyPrefix(n.desc.ID, n.index.ID))
}
if len(n.endKey) == 0 {
n.endKey = n.startKey.PrefixEnd()
if len(n.spans) == 0 {
// If no spans were specified retrieve all of the keys that start with our
// index key prefix.
start := proto.Key(MakeIndexKeyPrefix(n.desc.ID, n.index.ID))
n.spans = append(n.spans, span{
start: start,
end: start.PrefixEnd(),
})
}

// Retrieve all the spans.
b := &client.Batch{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's going to be a subtle interaction with LIMIT here when we implement that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you worried about?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a case like WHERE a IN (1,2,3,4,5,6,7) LIMIT 1 batching all the span reads will guarantee that the average cost of the query is also the worst-case cost. We'll want to do something more clever by reading N spans at a time where N is some derivative of the LIMIT number.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm aware of that. I thought the TODO above this method already captured this. I seem to have written that TODO multiple times so didn't think it was necessary calling out LIMIT here. Note that the limiting the rows read is only possible if the scan order matches the requested order.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, github hid that TODO from me. Yep, that's adequate, thanks.

if n.reverse {
n.kvs, n.err = n.txn.ReverseScan(n.startKey, n.endKey, 0)
for i := len(n.spans) - 1; i >= 0; i-- {
b.ReverseScan(n.spans[i].start, n.spans[i].end, 0)
}
} else {
n.kvs, n.err = n.txn.Scan(n.startKey, n.endKey, 0)
for i := 0; i < len(n.spans); i++ {
b.Scan(n.spans[i].start, n.spans[i].end, 0)
}
}
if n.err != nil {
if n.err = n.txn.Run(b); n.err != nil {
return false
}

for _, result := range b.Results {
if n.kvs == nil {
n.kvs = result.Rows
} else {
n.kvs = append(n.kvs, result.Rows...)
}
}

// Prepare our index key vals slice.
if n.valTypes, n.err = makeKeyVals(n.desc, n.columnIDs); n.err != nil {
return false
Expand Down
Loading