The normal way to accomplish something like this in golang would be to use a context.Context for the work that you are dispatching, then cancelling the context when you want to abandon the rest of the work.

It would also make sense to have the workers fan in their results to a goroutine that handles results and decides when to cancel.

Indeed, I understand what you mean, maybe a callback function can do the same, just like bsalamat said. And using Context may be too complicated for a public function.

Given that this is in client-go, I think we should make this a more generic function. More specifically, I think we should:

1. Rename it to ParallelizeUntil.
2. Change feasible *int32 to a function that returns a bool and an error, similar to ConditionFunc. When the return value is true, it stops.

Thanks for your kind review. :). I've updated it.

I suggest replacing ConditionFunc with a context.Context or stop channel. Then each worker just selects on toProcess and the done/stop channel.

s/allow/allows/

and I would appreciate it if you fix the same typo on Parallelize.

No problem.

Since there is a lot of duplicated code here, I suggest rewriting Parallelize to just call this function.

A good idea.

After this PR merge, I will submit a separate PR to replace all Parallelize.

I think there was a slight misunderstanding. We should only replace the definition of Parallelize so that existing callers won't see any change. Parallelize would just call ParallelizeUntil.

I think that is a small change that should be included in this PR.

I know a lot of this code was here before your change, but with this new ParallelizeUntil function, I think we can make this code much cleaner and easier to read.

Rather than having each worker write to an array and atomically increment and index variable, workers should send *v1.Node objects on a result channel. Then in this function, we can read from the result channel and build our filtered slice. Since only one goroutine can access filtered we can remove the synchronization around that.

I have a question and need your guidance. If pass the channel instead of synchronization as you said, is it also needed for errs and failedPredicateMap?

And the size of the channel buffer is also a problem that needs to be determined.

I have to say, it is a little weird that we are building a new util which passes integers to the worker funcs when our specific use case ignores those integers.

Since golang doesn't have generics, the more "canonical" way to solve that would be to have the worker funcs capture their work channel in a closure so that the utility doesn't have to manage that. What do you think?

Yes, I agree with you. We should pass in a channel, and then each worker function only needs data from the goroutine (here is one *v1.Node). But do we need to build this channel first? Because this may be a channel with a large size, this is my doubt.

what happen if err is NOT nil?

btw, as an util, do we accept nil conditionFunc ?

Yes,we should handle it. I'll follow misterikkit's comment and update it later.

Do we need remainingNodeNumber? I think we can use filteredLen and stop searching for more nodes once filteredLen >= numNodesToFind.

If we re-write this if statement as below, we can change line 377 to filtered = make([]*v1.Node, numNodesToFind). This saves us some memory.

if fits {
	len := atomic.AddInt32(&filteredLen, 1)
	if len > numNodesToFind {
		cancel()
	} else {
		filtered[len - 1] = g.cachedNodeInfoMap[nodeName].Node()		
	}
}

DONE.

We will not add to filtered if the index is larger than numNodesToFind, so we should be fine with allocating only numNodesToFind entries. If you still insist, please create a const, like numWorkers, and use it here and in ParallelizeUntil. In a few years, no body would know why this +16 is here.

I'm sorry,this is what I tested and I forgot to delete it.

maybe change to return Parallelize(workers, pieces, doWorkPiece) so that we don't need to check if context is done in select{} for this "ctx == nil" case

Per other comments, I think the plan is for Parallelize to call this func with a nil context to eliminate duplicate code.

context.TODO() implies that we need further code changes to get an appropriate context. Since your goal is to create a context that never cancels, it is more appropriate to use context.Background().

However, I think it would be cleaner to write it as,

var stop chan struct{}
if ctx != nil {
    stop = ctx.Done()
}

This is not a good idea, ParallelizeUntil should not rely on Parallelize.Because ParallelizeUntil may replace Parallelize.

That's fair.

Suggest rewording "until the context done or canceled" to "until done or the context is canceled"

Per other comments, I think the plan is for Parallelize to call this func with a nil context to eliminate duplicate code.

context.TODO() implies that we need further code changes to get an appropriate context. Since your goal is to create a context that never cancels, it is more appropriate to use context.Background().

However, I think it would be cleaner to write it as,

var stop chan struct{}
if ctx != nil {
    stop = ctx.Done()
}

This comment needs updating, since remainingNodesNumber is no longer used.

Done, thanks.

nit: would leave these lines.