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

[no-release-notes] Change some tests to use t.TempDir instead of os.TempDir so that their temporary files get cleaned up. #8792

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

reltuk
Copy link
Contributor

@reltuk reltuk commented Jan 25, 2025

No description provided.

…empDir so that their temporary files get cleaned up.
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
41fe1ac ok 5937457
version total_tests
41fe1ac 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
742f276 ok 5937457
version total_tests
742f276 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
be448ee ok 5937457
version total_tests
be448ee 5937457
correctness_percentage
100.0

Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

fulghum and others added 22 commits January 31, 2025 13:59
…equests so that a GC does not end while they are in progress.
…chunks which were in the memtable but get filtered out because they are already in the store.
…h for chunks that are written but which are already present in the store.
…g, make it so that reading chunks as part of the GC process does not take further dependencies on them and never blocks on waitForGC.
…callbacks to the replica controller execution context session.

This makes small clean ups to the lifecycle around replica applier and the
ownership and lifecycle of the mysql.Conn read connection.

This PR also includes some changes to slightly improve the performance and
reliability of the tests when running them locally. In particular, some of the
changes include:

1) Since `go run ./cmd/dolt` takes about four seconds to validate the existing
   cached build on my laptop, we just go ahead and use a cached build everywhere.

2) We use t.Log{f,} instead of fmt.Prin.. to improve the ergonomics of test
   running and getting output from a failure in particular.

3) We try to minimize global process state changes like unnecessary `os.Chdir`
   calls, since it would be nice to parallelize these tests eventually.

4) We get rid of the unused and seemingly unnecessary --socket= argument to
   Dolt, where we had to use a directory not corresponding to $TMPDIR, for
   example, because max pathlength on a sun_path on MacOS is 104 characters or
   whatever.
coffeegoddd and others added 9 commits January 31, 2025 13:59
…System, remove the confusing indirection through ctxFactory.
….Contexts used in remotesrv RPCs.

This PR changes each RPC invocation against the gRPC and HTTP servers
implementing remotesapi and cluster replication to create a sql.Context
which lives the duration of the call. The Session for that call gets
SessionCommand{Begin,End} and SessionEnd lifecycle callbacks so that it
can participate in GC safepoint rendezvous appropriately.

Previously the remotesrv.DBCache and the user/password remotesapi
authentication implementation would simply create new sql.Contexts
whenever they needed them. There could be multiple sql.Contexts for the
single server call.
Co-authored-by: Maximilian Hoffman <max@dolthub.com>
@coffeegoddd
Copy link
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
29cb66e ok 5937457
version total_tests
29cb66e 5937457
correctness_percentage
100.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants