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.

Sign up for GitHub

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

Jump to bottom

Maximum allowed writes exceeded in Firestore Commit #12007

Closed
silamon opened this issue Apr 15, 2022 · 2 comments · Fixed by #12111
Closed

Maximum allowed writes exceeded in Firestore Commit #12007

silamon opened this issue Apr 15, 2022 · 2 comments · Fixed by #12111
Assignees
@rosstimothy
Labels
bug

Comments

@silamon
Copy link

silamon commented Apr 15, 2022

Description

What happened:
Teleport tries to purge more than 500 expired documents and reaches a limit of Firestore documented here:
https://firebase.google.com/docs/firestore/quotas#writes_and_transactions

│ ERROR REPORT:                                                                                                                                                           │
│ Original Error: *status.Error rpc error: code = InvalidArgument desc = maximum 500 writes allowed per request                                                           │
│ Stack Trace:                                                                                                                                                            │
│     /go/src/github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:716 github.com/gravitational/teleport/lib/backend/firestore.ConvertGRPCError         │
│     /go/src/github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:694 github.com/gravitational/teleport/lib/backend/firestore.(*Backend).purgeExpiredD │
│     /go/src/github.com/gravitational/teleport/lib/backend/firestore/firestorebk.go:583 github.com/gravitational/teleport/lib/backend/firestore.RetryingAsyncFunctionRun │
│     /opt/go/src/runtime/asm_amd64.s:1371 runtime.goexit                                                                                                                 │
│ User Message: rpc error: code = InvalidArgument desc = maximum 500 writes allowed per request] firestore/firestorebk.go:585

Related code: https://github.com/gravitational/teleport/blob/master/lib/backend/firestore/firestorebk.go#L709-L721

What you expected to happen:
A commit should not contain more than 500 documents. If more than 500 documents are ready to be purged, multiple commits should be made.

Reproduction Steps

As minimally and precisely as possible, describe step-by-step how to reproduce the problem.

  1. Setup Teleport with Firestore backend
  2. Have more than 500 expired audit events
  3. Let Teleport try to purge them

Server Details

  • Teleport version (run teleport version): 7.3.17
  • Server OS (e.g. from /etc/os-release):
  • Where are you running Teleport? (e.g. AWS, GCP, Dedicated Hardware): GCP
  • Additional details:
@silamon silamon added the bug label Apr 15, 2022
rosstimothy added a commit that referenced this issue Apr 20, 2022
@rosstimothy
Respect Firestore commit write limits
0d9b63d 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007
@rosstimothy rosstimothy mentioned this issue Apr 20, 2022
Merged
rosstimothy added a commit that referenced this issue Apr 21, 2022
@rosstimothy
Respect Firestore commit write limits (#12111)
b543b99 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007
@silamon
Copy link
Author

silamon commented Apr 22, 2022

@rosstimothy, thank you for taking the time to fix this issue. Very much appreciated! Would it be possible to have this backported to v7 or v8?

rosstimothy added a commit that referenced this issue Apr 22, 2022
@rosstimothy
Respect Firestore commit write limits (#12111)
8eea481 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)

# Conflicts:
#	lib/backend/firestore/firestorebk_test.go
rosstimothy added a commit that referenced this issue Apr 22, 2022
@rosstimothy
Respect Firestore commit write limits (#12111)
3dc9e99 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)

# Conflicts:
#	lib/backend/firestore/firestorebk_test.go
rosstimothy added a commit that referenced this issue Apr 22, 2022
@rosstimothy
Respect Firestore commit write limits (#12111)
98e8766 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)
@rosstimothy
Copy link
Contributor

@rosstimothy, thank you for taking the time to fix this issue. Very much appreciated! Would it be possible to have this backported to v7 or v8?

Backport PRs are now up for v7, v8, v9. Sorry I was a bit busy and didn't get to this yesterday.

r0mant pushed a commit that referenced this issue Apr 22, 2022
@rosstimothy
Respect Firestore commit write limits (#12111) (#12177)
fccc509 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)
rosstimothy added a commit that referenced this issue Apr 25, 2022
@rosstimothy
Respect Firestore commit write limits (#12111) (#12176)
14b3976 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)

# Conflicts:
#	lib/backend/firestore/firestorebk_test.go
rosstimothy added a commit that referenced this issue Apr 25, 2022
@rosstimothy
Respect Firestore commit write limits (#12111) (#12175)
48e23c4 
Firestore only allows 500 writes per commit. Prior to this change
DeleteRange or purgeExpiredDocuments could result in deleting
more than 500 documents and would result in an error. To comply with
the limits both DeletRange and purgeExpiredDocuments now call
deleteDocuments which properly handles limiting a batch to 500 writes
and commiting multiple times until all documents have been removed.

Fixes #12007

(cherry picked from commit b543b99)

# Conflicts:
#	lib/backend/firestore/firestorebk_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rosstimothy rosstimothy

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Respect Firestore commit write limits gravitational/teleport
2 participants
@silamon @rosstimothy