search

openark / orchestrator

MySQL replication topology management and HA
Apache License 2.0
5.63k stars 931 forks source link

about queue len #909

Open yangeagle opened 5 years ago

yangeagle commented 5 years ago

file:go/discovery/queue.go

code:

// QueueLen returns the length of the queue (channel size + queued size)
func (q *Queue) QueueLen() int {
  q.Lock()
  defer q.Unlock()

  return len(q.queue) + len(q.queuedKeys)
}

I am confused about the calculation of the queue length.

q.queue and q.queuedKeys are the same.

Is there double counting here?

Thank you!

@MOON-CLJ

shlomi-noach commented 5 years ago

cc @sjmudd who authored this code.

MOON-CLJ commented 4 years ago

@shlomi-noach @sjmudd any update? It obviously feels like a bug.

sjmudd commented 4 years ago

This is code that's been around for a long time but after looking again I think I would agree. I am tempted to modify this however to reflect the complete length of the discovery queue. That is both the pending (queued) plus being processed (consumed) entries as that is the total size.

e.g.

return len(q.queuedKeys) + len(q.consumedKeys)

So prior to making any changes I wonder what you were looking for?

MOON-CLJ commented 4 years ago

@sjmudd separation's good.

sjmudd commented 4 years ago

@MOON-CLJ: thanks for catching this. I've made a PR which I think should address the issue.