| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
lgtm
Sorry, something went wrong.
Sorry, something went wrong.
|
Weird. Now Windows is failing because the tmp directory cleanup is failing with EPERM. |
Sorry, something went wrong.
I'm a bit worried that #53617 may have introduced a subtle change in behaviour/regression. |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
The current test is large and can time out. It should be split into multiple smaller tests as done in nodejs#54014. However, that approach appears to change GC behavior such that the database files are not cleaned up quickly enough on Windows. Forcing any unfinalized SQL statements to be GC'ed appears to fix the problem. Mark the original test as flaky until the necessary code changes are made.
|
The issue with these smaller tests appears to be related to relying on GC to finalize the prepared statements and close the database connections. Fixing it properly will take a little more time, so I have opened #54031 to mark the original test as flaky in the mean time. |
Sorry, something went wrong.
The current test is large and can time out. It should be split into multiple smaller tests as done in #54014. However, that approach appears to change GC behavior such that the database files are not cleaned up quickly enough on Windows. Forcing any unfinalized SQL statements to be GC'ed appears to fix the problem. Mark the original test as flaky until the necessary code changes are made. PR-URL: #54031 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Richard Lau <rlau@redhat.com>
The current test is large and can time out. It should be split into multiple smaller tests as done in #54014. However, that approach appears to change GC behavior such that the database files are not cleaned up quickly enough on Windows. Forcing any unfinalized SQL statements to be GC'ed appears to fix the problem. Mark the original test as flaky until the necessary code changes are made. PR-URL: #54031 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Richard Lau <rlau@redhat.com>
The current test is large and can time out. It should be split into multiple smaller tests as done in #54014. However, that approach appears to change GC behavior such that the database files are not cleaned up quickly enough on Windows. Forcing any unfinalized SQL statements to be GC'ed appears to fix the problem. Mark the original test as flaky until the necessary code changes are made. PR-URL: #54031 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Richard Lau <rlau@redhat.com>
|
There have been so many problems lately with EPERM on Windows that I'm not sure the failures on Windows were ever related to the changes in this PR. Starting the CI all over again to see how it goes. |
Sorry, something went wrong.
Sorry, something went wrong.
Codecov ReportAttention: Patch coverage is 73.58491% with 14 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## main #54014 +/- ##
==========================================
- Coverage 87.10% 87.10% -0.01%
==========================================
Files 647 647
Lines 181739 181775 +36
Branches 34887 34897 +10
==========================================
+ Hits 158310 158337 +27
+ Misses 16738 16736 -2
- Partials 6691 6702 +11
|
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
|
I've made the changes I outlined in #54014 (comment). This is now more than just a test change. The CI seems happy (still resuming some unrelated failures). I would appreciate re-reviews. |
Sorry, something went wrong.
Sorry, something went wrong.
This commit adds statement tracking to the DatabaseSync class. When a database is closed manually or via garbage collection, it will force all associated prepared statements to be finalized. This should mitigate "zombie" connections which can introduce test flakiness in the CI on Windows. PR-URL: #54014 Fixes: #54006 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
The original test/parallel/test-sqlite.js test appears to time out in the CI occasionally. This commit splits the test into several smaller test files. Fixes: #54006 PR-URL: #54014 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #54014 Fixes: #54006 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This commit adds statement tracking to the DatabaseSync class. When a database is closed manually or via garbage collection, it will force all associated prepared statements to be finalized. This should mitigate "zombie" connections which can introduce test flakiness in the CI on Windows. PR-URL: #54014 Fixes: #54006 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
The original test/parallel/test-sqlite.js test appears to time out in the CI occasionally. This commit splits the test into several smaller test files. Fixes: #54006 PR-URL: #54014 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #54014 Fixes: #54006 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
The current test is large and can time out. It should be split into multiple smaller tests as done in #54014. However, that approach appears to change GC behavior such that the database files are not cleaned up quickly enough on Windows. Forcing any unfinalized SQL statements to be GC'ed appears to fix the problem. Mark the original test as flaky until the necessary code changes are made. PR-URL: #54031 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Richard Lau <rlau@redhat.com>
| Back | FazBrowse Home | New Git URL |
The original test/parallel/test-sqlite.js test appears to time out in the CI occasionally. This commit splits the test into several smaller test files.
Fixes: #54006