Skip to content

Commit 2ecd4fc

Browse files
committed
try to reuse the info.Queue conversion has a negative performance effect
1 parent 1edd031 commit 2ecd4fc

File tree

2 files changed

+49
-20
lines changed

2 files changed

+49
-20
lines changed

txn/flusher.go

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -159,33 +159,41 @@ const preloadBatchSize = 100
159159

160160
func (f *flusher) recurse(t *transaction, seen map[bson.ObjectId]*transaction, preloaded map[bson.ObjectId]*transaction) error {
161161
seen[t.Id] = t
162+
// we shouldn't need this one anymore because we are processing it now
162163
delete(preloaded, t.Id)
163164
err := f.advance(t, nil, false)
164165
if err != errPreReqs {
165166
return err
166167
}
168+
toPreload := make([]bson.ObjectId, 0)
167169
for _, dkey := range t.docKeys() {
168-
remaining := make([]bson.ObjectId, 0, len(f.queue[dkey]))
169-
toPreload := make(map[bson.ObjectId]struct{}, len(f.queue[dkey]))
170+
queue := f.queue[dkey]
171+
remaining := make([]bson.ObjectId, 0, len(queue))
170172
for _, dtt := range f.queue[dkey] {
171173
id := dtt.id()
172-
if _, scheduled := toPreload[id]; seen[id] != nil || scheduled || preloaded[id] != nil {
174+
if seen[id] != nil {
173175
continue
174176
}
175-
toPreload[id] = struct{}{}
176177
remaining = append(remaining, id)
177178
}
178-
// done with this map
179-
toPreload = nil
179+
180180
for len(remaining) > 0 {
181-
batch := remaining
182-
if len(batch) > preloadBatchSize {
183-
batch = remaining[:preloadBatchSize]
181+
toPreload = toPreload[:0]
182+
batchSize := preloadBatchSize
183+
if batchSize > len(remaining) {
184+
batchSize = len(remaining)
184185
}
185-
remaining = remaining[len(batch):]
186-
err := f.loadMulti(batch, preloaded)
187-
if err != nil {
188-
return err
186+
batch := remaining[:batchSize]
187+
remaining = remaining[batchSize:]
188+
for _, id := range batch {
189+
if preloaded[id] == nil {
190+
toPreload = append(toPreload, id)
191+
}
192+
}
193+
if len(toPreload) > 0 {
194+
if err := f.loadMulti(toPreload, preloaded); err != nil {
195+
return err
196+
}
189197
}
190198
for _, id := range batch {
191199
if seen[id] != nil {
@@ -302,6 +310,8 @@ NextDoc:
302310
if info.Remove == "" {
303311
// Fast path, unless workload is insert/remove heavy.
304312
revno[dkey] = info.Revno
313+
// We updated the Q, so this should force refresh
314+
// TODO: We could *just* add the new txn-queue entry/reuse existing tokens
305315
f.queue[dkey] = tokensWithIds(info.Queue)
306316
f.debugf("[A] Prepared document %v with revno %d and queue: %v", dkey, info.Revno, info.Queue)
307317
continue NextDoc
@@ -363,8 +373,14 @@ NextDoc:
363373
} else {
364374
f.debugf("[B] Prepared document %v with revno %d and queue: %v", dkey, info.Revno, info.Queue)
365375
}
366-
revno[dkey] = info.Revno
367-
f.queue[dkey] = tokensWithIds(info.Queue)
376+
existRevno, rok := revno[dkey]
377+
existQ, qok := f.queue[dkey]
378+
if rok && qok && existRevno == info.Revno && len(existQ) == len(info.Queue) {
379+
// We've already loaded this doc, no need to load it again
380+
} else {
381+
revno[dkey] = info.Revno
382+
f.queue[dkey] = tokensWithIds(info.Queue)
383+
}
368384
continue NextDoc
369385
}
370386
}
@@ -498,15 +514,25 @@ func (f *flusher) rescan(t *transaction, force bool) (revnos []int64, err error)
498514
goto RetryDoc
499515
}
500516
revno[dkey] = info.Revno
501-
517+
// TODO(jam): 2017-05-31: linear search for each token in info.Queue during all rescans is potentially O(N^2)
518+
// if we first checked to see that we've already loaded this info.Queue in f.queue, we could use a different
519+
// structure (map) to do faster lookups to see if the tokens are already present.
502520
found := false
503521
for _, id := range info.Queue {
504522
if id == tt {
505523
found = true
506524
break
507525
}
508526
}
509-
f.queue[dkey] = tokensWithIds(info.Queue)
527+
// f.queue[dkey] = tokensWithIds(info.Queue, &RescanUpdatedQueue)
528+
existQ, qok := f.queue[dkey]
529+
if qok && len(existQ) == len(info.Queue) {
530+
// we could check that info.Q matches existQ.tt
531+
} else {
532+
if len(existQ) != len(info.Queue) {
533+
}
534+
f.queue[dkey] = tokensWithIds(info.Queue)
535+
}
510536
if !found {
511537
// Rescanned transaction id was not in the queue. This could mean one
512538
// of three things:

txn/txn.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,10 @@ func newNonce() string {
136136

137137
type token string
138138

139-
func (tt token) id() bson.ObjectId { return bson.ObjectIdHex(string(tt[:24])) }
140-
func (tt token) nonce() string { return string(tt[25:]) }
139+
func (tt token) id() bson.ObjectId {
140+
return bson.ObjectIdHex(string(tt[:24]))
141+
}
142+
func (tt token) nonce() string { return string(tt[25:]) }
141143

142144
// Op represents an operation to a single document that may be
143145
// applied as part of a transaction with other operations.
@@ -330,6 +332,8 @@ func (r *Runner) ResumeAll() (err error) {
330332
panic(fmt.Errorf("invalid state for %s after flush: %q", &t, t.State))
331333
}
332334
}
335+
// TODO(jam): 2017-06-04 This is not calling iter.Close() and dealing with
336+
// any error it might encounter (db connection closed, etc.)
333337
return nil
334338
}
335339

@@ -478,7 +482,6 @@ func (r *Runner) loadMulti(ids []bson.ObjectId, preloaded map[bson.ObjectId]*tra
478482
return nil
479483
}
480484

481-
482485
type typeNature int
483486

484487
const (

0 commit comments

Comments
 (0)