Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module leases fail to expire #2215

Closed
matt2e opened this issue Jul 31, 2024 · 1 comment · Fixed by #2265
Closed

Module leases fail to expire #2215

matt2e opened this issue Jul 31, 2024 · 1 comment · Fixed by #2265
Assignees
Labels
bug Something isn't working P1

Comments

@matt2e
Copy link
Collaborator

matt2e commented Jul 31, 2024

Steps to reproduce:

  • Alter examples/go/time/time.go to obtain a lease but forget to release it:
func Time(ctx context.Context, req TimeRequest) (TimeResponse, error) {
   _, err := ftl.Lease(ctx, 10*time.Second, "time")
   if err != nil {
   	return TimeResponse{}, fmt.Errorf("could not obtain time lease: %w", err)
   }
   return TimeResponse{Time: time.Now()}, nil
}
  • Go to the console and trigger the time.time verb. Do it once a second...

Expected result:

  • The first call to time.time should succeed
  • Subsequent calls to time.time should fail with could not obtain time lease for the next 10 seconds or so (waiting for the lease to expire)

Actual result:

  • First call succeeds
  • Subsequent calls fail indefinitely...
  • Trigger a new deploy of time by modifying any code in time.go
  • Taking down the old deployment will cause the lease to finally expire
  • Next call to time.time will succeed, and then the cycle continues...

From the digging I've done, here's why:

Leaving that connection alive sounds like a resource leak. Is making sure that gets closed enough to make this safe, or should the controller also only heartbeat the lease when it receives the heartbeat message from the module?

@github-actions github-actions bot added the triage Issue needs triaging label Jul 31, 2024
@ftl-robot ftl-robot mentioned this issue Jul 31, 2024
@alecthomas
Copy link
Collaborator

I might be misunderstanding, but this is user error, not a bug. Leases must be released, similarly to how file handles must be closed.

@jonathanj-square jonathanj-square added bug Something isn't working P1 next Work that will be be picked up next labels Jul 31, 2024
@github-actions github-actions bot removed the triage Issue needs triaging label Jul 31, 2024
@matt2e matt2e self-assigned this Aug 6, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Aug 6, 2024
matt2e added a commit that referenced this issue Aug 7, 2024
fixes #2215

When a module lease has an error (example ctx canceled due to the verb
call ending), it used to leave the stream with the controller open
forever. This change closes that stream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants