Remove BacklogManager from AckManager

[stephanos]

What changed?

Removes the reference to BacklogManager from AckManager.

Why?

Removes circular dependency.

How did you test it?

Existing tests. No behavior change is expected.

Potential risks

Documentation

Is hotfix candidate?

[ychebotarev]

not a part of this PR.
I see logger is only used in func (m *ackManager) addTask.
logger is used as logger.Fatal, but execution continues without returning any error.
it is used in only in func (tr *taskReader) addSingleTaskToBuffer(:
tr.backlogMgr.taskAckManager.addTask(task.GetTaskId())

which does return error. Is that intended?
should addTask continue in those logger.Fatal cases like "Already present in outstanding tasks"?

[stephanos]

Good question, Yuri; cc @ShahabT ?

[ShahabT]

cc @dnr to answer the question about addTask fatal log.

[dnr]

I'm not sure I understand the question, but:

Those Fatal logs are invariants that indicate a bug in code using ackManager. Possibly there is some way to recover if the invariant is violated but given that there is currently no bug in the usage of ackManager (that I know of), I don't think we should relax the checks to recover better from a future hypothetical bug.

Also note that addSingleTaskToBuffer and addTasksToBuffer technically return an error, but getTasksPump doesn't actually handle an error return in any reasonable way (it just ignores it, with a comment saying only context errors are possible, which is true now), so it's not really true that we could recover some correct behavior by returning an error there instead of log.Fatal.