gitserver: Remove effectively disabled debouncing of updates (#60584)

The only value we ever passed here was 1 second, which is effectively "don't debounce".
So we can simplify here.

Really, the client should make sure to go at a reasonable pace, and gitserver on the
other end of the wire can have ultimate authority over if it's time for an update or
not.

All existing tests still pass, integration and E2E tests are passing.

Author: eseliger

cmd/gitserver/internal/integration_tests/clone_test.go

@@ -88,7 +88,7 @@ func TestClone(t *testing.T) {
 
 	// Requesting a repo update should figure out that the repo is not yet
 	// cloned and call clone. We expect that clone to succeed.
-	_, err := cli.RequestRepoUpdate(ctx, repo, 0)
+	_, err := cli.RequestRepoUpdate(ctx, repo)
 	require.NoError(t, err)
 
 	// Should have acquired a lock.
@@ -185,7 +185,7 @@ func TestClone_Fail(t *testing.T) {
 	// Requesting a repo update should figure out that the repo is not yet
 	// cloned and call clone. We expect that clone to fail, because vcssyncer.IsCloneable
 	// fails here.
-	resp, err := cli.RequestRepoUpdate(ctx, repo, 0)
+	resp, err := cli.RequestRepoUpdate(ctx, repo)
 	require.NoError(t, err)
 	// Note that this error is from IsCloneable(), not from Clone().
 	require.Contains(t, resp.Error, "error cloning repo: repo github.com/test/repo not cloneable:")
@@ -223,7 +223,7 @@ func TestClone_Fail(t *testing.T) {
 	// Requesting another repo update should figure out that the repo is not yet
 	// cloned and call clone. We expect that clone to fail, but in the vcssyncer.Clone
 	// stage this time, not vcssyncer.IsCloneable.
-	resp, err = cli.RequestRepoUpdate(ctx, repo, 0)
+	resp, err = cli.RequestRepoUpdate(ctx, repo)
 	require.NoError(t, err)
 	require.Contains(t, resp.Error, "failed to clone github.com/test/repo: clone failed. Output: Creating bare repo\nCreated bare repo at")
 

cmd/gitserver/internal/integration_tests/resolverevisions_test.go

@@ -103,7 +103,7 @@ func TestClient_ResolveRevision(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run("", func(t *testing.T) {
-			_, err := cli.RequestRepoUpdate(ctx, api.RepoName(remote), 0)
+			_, err := cli.RequestRepoUpdate(ctx, api.RepoName(remote))
 			require.NoError(t, err)
 
 			got, err := cli.ResolveRevision(ctx, api.RepoName(remote), test.input, gitserver.ResolveRevisionOptions{NoEnsureRevision: true})

cmd/gitserver/internal/integration_tests/test_utils.go

@@ -117,7 +117,7 @@ func MakeGitRepository(t testing.TB, cmds ...string) api.RepoName {
 	t.Helper()
 	dir := InitGitRepository(t, cmds...)
 	repo := api.RepoName(filepath.Base(dir))
-	if resp, err := testGitserverClient.RequestRepoUpdate(context.Background(), repo, 0); err != nil {
+	if resp, err := testGitserverClient.RequestRepoUpdate(context.Background(), repo); err != nil {
 		t.Fatal(err)
 	} else if resp.Error != "" {
 		t.Fatal(resp.Error)

cmd/gitserver/internal/server.go

@@ -57,25 +57,6 @@ import (
 // logs to stderr
 var traceLogs bool
 
-var (
-	lastCheckAt    = make(map[api.RepoName]time.Time)
-	lastCheckMutex sync.Mutex
-)
-
-// debounce() provides some filtering to prevent spammy requests for the same
-// repository. If the last fetch of the repository was within the given
-// duration, returns false, otherwise returns true and updates the last
-// fetch stamp.
-func debounce(name api.RepoName, since time.Duration) bool {
-	lastCheckMutex.Lock()
-	defer lastCheckMutex.Unlock()
-	if t, ok := lastCheckAt[name]; ok && time.Now().Before(t.Add(since)) {
-		return false
-	}
-	lastCheckAt[name] = time.Now()
-	return true
-}
-
 func init() {
 	traceLogs, _ = strconv.ParseBool(env.Get("SRC_GITSERVER_TRACE", "false", "Toggles trace logging to stderr"))
 }
@@ -539,14 +520,11 @@ func (s *Server) RepoUpdate(ctx context.Context, req *protocol.RepoUpdateRequest
 		return resp
 	}
 
-	var statusErr, updateErr error
-
-	if debounce(req.Repo, req.Since) {
-		updateErr = s.doRepoUpdate(ctx, req.Repo, "")
-	}
+	updateErr := s.doRepoUpdate(ctx, req.Repo, "")
 
 	// attempts to acquire these values are not contingent on the success of
 	// the update.
+	var statusErr error
 	lastFetched, err := repoLastFetched(dir)
 	if err != nil {
 		statusErr = err

cmd/repo-updater/internal/scheduler/scheduler.go

@@ -179,7 +179,7 @@ func (s *UpdateScheduler) runUpdateLoop(ctx context.Context) {
 				// if it doesn't exist or update it if it does. The timeout of this request depends
 				// on the value of conf.GitLongCommandTimeout() or if the passed context has a set
 				// deadline shorter than the value of this config.
-				resp, err := s.gitserverClient.RequestRepoUpdate(ctx, repo.Name, 1*time.Second)
+				resp, err := s.gitserverClient.RequestRepoUpdate(ctx, repo.Name)
 				if err != nil {
 					schedError.WithLabelValues("requestRepoUpdate").Inc()
 					subLogger.Error("error requesting repo update", log.Error(err), log.String("uri", string(repo.Name)))

cmd/repo-updater/internal/scheduler/scheduler_test.go

@@ -1428,7 +1428,7 @@ func TestUpdateScheduler_runUpdateLoop(t *testing.T) {
 			contexts := make(chan context.Context, expectedRequestCount)
 			db := dbmocks.NewMockDB()
 			gs := gitserver.NewMockClient()
-			gs.RequestRepoUpdateFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName, d time.Duration) (*gitserverprotocol.RepoUpdateResponse, error) {
+			gs.RequestRepoUpdateFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName) (*gitserverprotocol.RepoUpdateResponse, error) {
 				select {
 				case mock := <-mockRequestRepoUpdates:
 					if !reflect.DeepEqual(mock.repo.Name, repo) {

internal/batches/sources/mocks_test.go

@@ -363,9 +363,8 @@ type Client interface {
 	// with more detailed responses. Do not use this if you are not repo-updater.
 	//
 	// Repo updates are not guaranteed to occur. If a repo has been updated
-	// recently (within the Since duration specified in the request), the
-	// update won't happen.
-	RequestRepoUpdate(context.Context, api.RepoName, time.Duration) (*protocol.RepoUpdateResponse, error)
+	// recently, the update won't happen.
+	RequestRepoUpdate(context.Context, api.RepoName) (*protocol.RepoUpdateResponse, error)
 
 	// RequestRepoClone is an asynchronous request to clone a repository.
 	RequestRepoClone(context.Context, api.RepoName) (*protocol.RepoCloneResponse, error)
@@ -707,19 +706,17 @@ func (c *clientImplementor) gitCommand(repo api.RepoName, arg ...string) GitComm
 	}
 }
 
-func (c *clientImplementor) RequestRepoUpdate(ctx context.Context, repo api.RepoName, since time.Duration) (_ *protocol.RepoUpdateResponse, err error) {
+func (c *clientImplementor) RequestRepoUpdate(ctx context.Context, repo api.RepoName) (_ *protocol.RepoUpdateResponse, err error) {
 	ctx, _, endObservation := c.operations.requestRepoUpdate.With(ctx, &err, observation.Args{
 		MetricLabelValues: []string{c.scope},
 		Attrs: []attribute.KeyValue{
 			repo.Attr(),
-			attribute.Stringer("since", since),
 		},
 	})
 	defer endObservation(1, observation.Args{})
 
 	req := &protocol.RepoUpdateRequest{
-		Repo:  repo,
-		Since: since,
+		Repo: repo,
 	}
 
 	client, err := c.ClientForRepo(ctx, repo)

internal/gitserver/client.go

@@ -66,10 +66,9 @@ func TestClient_IsRepoCloneale_ProtoRoundTrip(t *testing.T) {
 func TestClient_RepoUpdateRequest_ProtoRoundTrip(t *testing.T) {
 	var diff string
 	t.Run("request", func(t *testing.T) {
-		fn := func(repo api.RepoName, since int64) bool {
+		fn := func(repo api.RepoName) bool {
 			original := protocol.RepoUpdateRequest{
-				Repo:  repo,
-				Since: time.Duration(since),
+				Repo: repo,
 			}
 
 			var converted protocol.RepoUpdateRequest