Repository: dolthub/dolt
File: go/store/datas/pull/pull_chunk_tracker.go (Go)

Code before:
			var thisHasManyReqCh chan trackerHasManyReq
			var hasManyReq trackerHasManyReq
			if len(unchecked) > 0 {
				hasManyReq.hs = unchecked[0]
				thisHasManyReqCh = hasManyReqCh
			}

			select {
			case h, ok := <-t.uncheckedCh:
				// uncheckedCh closing means the PullChunkTracker was closed.
				if !ok {
					return nil
				}
				// |h| is a hash we need to check against HasMany on the destination.
				// It could become a hash we fetch from source and add to the destination
				// or it could already be present.
				if len(unchecked) == 0 || len(unchecked[len(unchecked)-1]) >= t.cfg.BatchSize {
					outstanding += 1
					unchecked = append(unchecked, make(hash.HashSet))
				}
				unchecked[len(unchecked)-1].Insert(h)
			case resp := <-hasManyRespCh:
				// A hasMany response came back.
				outstanding -= 1
				if resp.err != nil {
					err = errors.Join(err, resp.err)
				}
				// Add all the resp.hs hashes, those which are not already present
				// in dest, to our batches of absent hashes we will return through
				// GetChunksToFetch.
				absent = appendAbsent(absent, resp.hs, t.cfg.BatchSize)
			case thisHasManyReqCh <- hasManyReq:
				// We delivered a hasMany request to a hasManyThread.
				// Remove it here. We do not need to update |outstanding|, since
				// it was updated when we created the new batch in |unchecked|.
				_, unchecked = pop(unchecked)
			case <-t.processedCh:
				// TickProcessed helps us keep track of chunks which we returned from GetChunksToFetch
				// and which are still being processed by WalkAddrs. This only gets called after all
				// unchecked hashes in the chunks have been delivered to us.
				unprocessed -= 1
			case req := <-thisReqCh:
				// A request for GetChunksToFetch.
				if err != nil {
					// Deliver an error we experienced. HasMany can error, and this is where
					// a client sees it, for example.
					req.err = err
					close(req.ready)
					err = nil
				} else if len(absent) == 0 {
					// We have no more chunks to deliver. If len(absent) == 0 but we had

Review comment (suggestion):
May want a continue statement here.  I assume that if resp.err is not nil then resp.hs should be empty, but I guess I'd still opt to skip the append statement as I don't know there are any guarantees that that is true.

Code after:
			var thisHasManyReqCh chan trackerHasManyReq
			var hasManyReq trackerHasManyReq
			if len(unchecked) > 0 {
				hasManyReq.hs = unchecked[0]
				thisHasManyReqCh = hasManyReqCh
			}

			select {
			case h, ok := <-t.uncheckedCh:
				// uncheckedCh closing means the PullChunkTracker was closed.
				if !ok {
					return nil
				}
				// |h| is a hash we need to check against HasMany on the destination.
				// It could become a hash we fetch from source and add to the destination
				// or it could already be present.
				if len(unchecked) == 0 || len(unchecked[len(unchecked)-1]) >= t.cfg.BatchSize {
					outstanding += 1
					unchecked = append(unchecked, make(hash.HashSet))
				}
				unchecked[len(unchecked)-1].Insert(h)
			case resp := <-hasManyRespCh:
				// A hasMany response came back.
				outstanding -= 1
				if resp.err != nil {
					err = errors.Join(err, resp.err)
				} else if len(resp.hs) > 0 {
					// Add all the resp.hs hashes, those which are not already present
					// in dest, to our batches of absent hashes we will return through
					// GetChunksToFetch.
					absent = appendAbsent(absent, resp.hs, t.cfg.BatchSize)
				}
			case thisHasManyReqCh <- hasManyReq:
				// We delivered a hasMany request to a hasManyThread.
				// Remove it here. We do not need to update |outstanding|, since
				// it was updated when we created the new batch in |unchecked|.
				_, unchecked = pop(unchecked)
			case <-t.processedCh:
				// TickProcessed helps us keep track of chunks which we returned from GetChunksToFetch
				// and which are still being processed by WalkAddrs. This only gets called after all
				// unchecked hashes in the chunks have been delivered to us.
				unprocessed -= 1
			case req := <-thisReqCh:
				// A request for GetChunksToFetch.
				if err != nil {
					// Deliver an error we experienced. HasMany can error, and this is where
					// a client sees it, for example.
					req.err = err
					close(req.ready)
					err = nil
				} else if len(absent) == 0 {