New concurrency strategy, tested against Datasette pre and post 1.0a7

[simonw]

• Improved compatibility with Datasette 1.0a+ #37

[simonw]

Weird, it's failing in CI against 1.0+ with this error:

>       assert rows == expected_rows
E       AssertionError: assert [{'age': '5',...: 'Pancakes'}] == [{'age': 5, '...: 'Pancakes'}]
E         At index 0 diff: {'name': 'Cleo', 'age': '5'} != {'name': 'Cleo', 'age': 5}
E         Full diff:
E         - [{'age': 5, 'name': 'Cleo'}, {'age': 4, 'name': 'Pancakes'}]
E         + [{'age': '5', 'name': 'Cleo'}, {'age': '4', 'name': 'Pancakes'}]
E         ?          + +                           + +

But passes on my laptop.

[simonw]

Since it's the type conversions that are failing I am suspicious of this code:

datasette-upload-csvs/datasette_upload_csvs/__init__.py

Lines 166 to 176 in a0b64f7

	def insert_docs_catch_errors(conn):
	database = sqlite_utils.Database(conn)
	try:
	insert_docs(database)
	except Exception as error:
	database["_csv_progress_"].update(
	task_id,
	{"error": str(error)},
	)
	await db.execute_write_fn(insert_docs_catch_errors, block=False)

Why is that block=False? The insert_docs() call inside of that uses TypeTracker to set the types.

[simonw]

Weird, still that same test failure where types are not converted on some Python versions for Datasette 1.0.

Even weirder: I saw it pass on 3.7, 3.10 and 3.11 with Datasette <1.0 in this run: https://github.com/simonw/datasette-upload-csvs/actions/runs/7658565625

But then when I added fail-fast: false it failed on everything except for 3.7 and 3.9: https://github.com/simonw/datasette-upload-csvs/actions/runs/7658713572?pr=36

Maybe a race condition or something else that's intermittent?

[simonw]

Some of those tests are taking way longer than they should, deadlock or race condition of some sort?

[simonw]

Looks like that is affecting ALL of the >=1.0a test runs:

[simonw]

Tests pass without that weird pause on my laptop against Datasette 1.0ax.

[simonw]

I'll try running this is Codespaces, see if I can recreate that weird testing bug.

[simonw]

In Codespaces the tests passed... but then it hung at the end of the test run rather than giving me back the terminal.

Hitting Ctrl+C there showed this error:

^CException ignored in: <module 'threading' from '/usr/local/python/3.10.13/lib/python3.10/threading.py'>
Traceback (most recent call last):
  File "/usr/local/python/3.10.13/lib/python3.10/threading.py", line 1537, in _shutdown
    atexit_call()
  File "/usr/local/python/3.10.13/lib/python3.10/concurrent/futures/thread.py", line 31, in _python_exit
    t.join()
  File "/usr/local/python/3.10.13/lib/python3.10/threading.py", line 1096, in join
    self._wait_for_tstate_lock()
  File "/usr/local/python/3.10.13/lib/python3.10/threading.py", line 1116, in _wait_for_tstate_lock
    if lock.acquire(block, timeout):
KeyboardInterrupt: 

[simonw]

Still a problem.

The Python jobs that are hung all seem to have run their test but then hung on quitting the pytest process.

[simonw]

OK, at least the tests fail after a minute now rather than hanging around for hours.

[simonw]

I'm going to try a different strategy: I'm going to run the CSV parsing in an async task on the main thread, but have it yield that task every 100 items inserted. Let's see if that works better.

Backup plan after that: could I run the CSV parsing in a separate thread but have 100 rows at a time passed from that thread back to a task in the main thread which then sends them to the write database?

[simonw]

OK, that async strategy seems to work pretty well! While uploading a large CSV file the app stayed responsive to other requests.

[simonw]

Test failures still look like this:

>       assert rows == expected_rows
E       AssertionError: assert [{'age': '5',...: 'Pancakes'}] == [{'age': 5, '...: 'Pancakes'}]
E         At index 0 diff: {'name': 'Cleo', 'age': '5'} != {'name': 'Cleo', 'age': 5}
E         Full diff:
E         - [{'age': 5, 'name': 'Cleo'}, {'age': 4, 'name': 'Pancakes'}]
E         + [{'age': '5', 'name': 'Cleo'}, {'age': '4', 'name': 'Pancakes'}]
E         ?          + +                           + +

Suggesting a timing issue where the types transform sometimes hasn't completed when the test runs.

[simonw]

Weird that the tests ONLY fail for Datasette >= 1.0a

[simonw]

That debug output looks correct to me:

Transforming types to {'IncidentNumber': 'integer', 'DateTimeOfCall': 'text', 'CalYear': 'integer', 'FinYear': 'text', 'TypeOfIncident': 'text', 'PumpCount': 'integer', 'PumpHoursTotal': 'integer', 'HourlyNotionalCost(£)': 'float', 'IncidentNotionalCost(£)': 'float'}

So why isn't that taking effect before the test assertion runs?

datasette-upload-csvs/tests/test_datasette_upload_csvs.py

Lines 167 to 186 in 233010e

	# Now things get tricky... the upload is running in a task, so poll for completion
	fail_after = 20
	iterations = 0
	while True:
	response = await client.get(
	"http://localhost/data/_csv_progress_.json?_shape=array"
	)
	rows = json.loads(response.content)
	assert 1 == len(rows)
	row = rows[0]
	assert row["table_name"] == expected_table
	assert not row["error"], row
	if row["bytes_todo"] == row["bytes_done"]:
	break
	iterations += 1
	assert iterations < fail_after, "Took too long: {}".format(row)
	await asyncio.sleep(0.5)
	rows = list(db[expected_table].rows)
	assert rows == expected_rows