Better handling of *.testproj directories as databases

[aeisenberg]

*.testproj databases are usually ephemeral. They are re-created by the test runner each time tests are run. This causes a problem when importing into vscode. When a database is changed on disk, the query server lock is not released. So, when the database is recreated, queries cannot be run since the original lock is invalid.

There are a few workarounds that users can do:

1. Before importing a testproj database, make sure to copy it, so it won't be deleted by the test runner.
2. Before rerunning tests, remove the database from the UI.

Neither of these solutions are ideal and the extension should handle this automatically. An ideal solution is that immediately before a test is run, check to see if the resulting database is already imported. If so, remove it and then run the tests and re-add when complete.

[adityasharad]

I like the idea of removing and deregistering the database when a test is run that would recreate it. Bonus points if we can readd the database again after the tests are done! Worth trying and seeing how our internal users find the experience.

[cklin]

I am unable to reproduce the issue as described. The steps I followed:

1. Run failing test from vscode
2. Import testproj directory as database
3. Change test
4. Rerun still-failing test from vscode
5. Run query from vscode against database

I am able to successfully run the query in step 5. Interestingly, the result of the query at step 5 reflects the state of the testproj directory at step 2 instead of at step 4.

I discussed with @aeisenberg, and we concluded that the behavior I observed is still worth fixing. Partly to make sure that the database is always up-to-date, and partly to deal with situations where the testproj directory is deleted by a successful test run.

[cklin]

I am also exploring an alternative solution that refreshes the database prior to each query. The benefit of doing so is that it solves the testproj staleness problem not only for tests initiated from the vscode extension, but also for tests initiated elsewhere. We can limit the automatic refresh to only those situations where the testproj directory has a new modification timestamp, so there should be very little wasted effort if the testproj directory is unchanged.

The same automatic refresh path can also handle situations where the testproj directory is removed by a successful test run.

I am currently in a deep dive trying to understand exactly what needs to be refreshed. If I unregister a testproj database and then reregister the same directory as database, new queries reflect the new state of the testproj directory. If I simply click the "Set Current Database" button for the testproj database, new queries would reflect the old (outdated) state of the testproj directory. Even though clicking "Set Current Database" calls refresh() on the DatabaseItem. So there is a difference between deregistering-reregistering and DatabaseItem.refresh(), and I want to find out what the difference is.

[cklin]

What works to make queries see current state of testproj directory:

• Restart Query Server
• Clear Cache

What does not work:

• Switch current database to another database and back

[aeisenberg]

One concern I have about restarting the query server is that there could be other tasks and queries running that would get cancelled. The query server can handle multiple requests simultaneously. A solution would be to wait until there are no more tasks running and then restart the query server, but that would make running a test essentially single threaded.

[aeisenberg]

* Switch current database to another database and back

What if you switch to another database, run a query, and then go back? There is an implementation detail that the query server doesn't release its old lock on the previous database until we do something with the current database.

Also, unfortunately, the DatabaseItemImpl.refresh() does something very different from (de-)registerDatabase. The former will ensure that the contents on the disk reflects what is displayed in the databases view in the UI. The latter will ensure that the database state is completely added to or removed from the query server.

It's the latter, and the helper reregisterDatabase, that will ensure that a new version of the same database can be seen in the query server.

[cklin]

The testing was only to help me pin down the persistent database state that needs to be cleared. I completely agree that restarting the query server is a poor solution to this issue, for all the reasons you described.

I did not test whether running a query in another database B helps with staleness in database A, as I do not anticipate being able to develop a solution based on that approach.

Asking the query server to clear cache for the database seems to work. So my current plan is:

• Clear cache before each query run, as proof-of-concept to see if that makes queries return fresh results
• Record the top-level modification timestamp (of zip archive or testproj directory) in DatabaseContents
• Clear cache before query run only when needed, i.e., when modification timestamp changes

Questions to be answered:

• Is clearing the cache the right approach? Or is it necessary to actually deregister and reregister the database? All else being equal, cache clearing seems more performant and less disruptive. Also potentially less complicated if we want to preserve database renaming.
• If cache clearing is sufficient, do we want to implement that change in the vscode extension? Or do we want to implement it in the query server, leaving the vscode extension entirely unchanged?

[aeisenberg]

I did not test whether running a query in another database B helps with staleness in database A, as I do not anticipate being able to develop a solution based on that approach.

Right...this was not a recommendation for how to implement, just a way to better explore what's happening and how database locks work.

Maybe @alexet could provide some insight as to whether or not clearing the cache would be sufficient. My guess is no, but I don't know for certain.

[alexet]

So the problem is very awakward. Once a database is open we assume that we are the only user and hence that we can write to it at any time. The way the queryserver works is that it assumes that any added database is exclusively owned. We do take a lock on the dtabase but because we access via paths but the lock is on a file, the lock doesn't prevent deleting (I think that the test runner should take a lock when deleting a database but that is a different issue). Clearing the caches means that any corrupt data is likely to have been deleted but it doesn't guarantee it as it relies on the caches being correct (i.e incorrect data will be deleted but we may lose track of things). None of this will work at all on windows where the lock prevents the database being deleted. Note that this is made more complex by the list of databases being things that the queryserver could lock rather than things it has locked. Also these are all basically fairly rare conditions so things will work 99% of the time but silently give you incorrect data every now and again.

There have been previous idea's about putting the database in a idle state without a lock at the end of queries so that it restarts when the underlying database changes but that involves some quite major evaluator changes as currently we flush aspects of the cache on shutdown so saving data and releasing memory are entwined (we use native memory in the evaluator so we cant just release our references). Also there is some data which we can't easily just close.

I think copying the database and using the copy and replacing the copy safely if it changes (i.e. deregister, recopy, register). We should also block running anything on the database while tests are running in the UI. That will prevent most half created databases from being an issue. This way we respect the query server rules while also doing what the user wants.

[cklin]

Thanks Alex. So copying the testproj directory and registering a database only from the copy should avoid test runs from interfering with queries. A few more questions:

• Any suggestions on where to store the copy?
• How much do we care about garbage collecting the testproj copies?
• Is it still necessary to block running anything on the database if the database is based on a testproj copy?