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

Fix garbled JSON-lines output sometimes #1040

Merged
merged 1 commit into from
Feb 6, 2025
Merged

Conversation

craigds
Copy link
Member

@craigds craigds commented Feb 6, 2025

Description

In kart diff -o json-lines --add-feature-count-estimate={value}, two threads would write to the output. This correctly used a lock to prevent races, but there is an intermediate bytearray which is written just beforehand (introduced with #1025) and this also needed to use a lock to prevent races.

This change moves that intermediate bytearray under the same lock as writing the output buffer, thus avoiding the race condition

Related links:

Checklist:

  • Have you reviewed your own change?
  • Have you included test(s)?
  • Have you updated the changelog?

In `kart diff -o json-lines --add-feature-count-estimate={value}`,
two threads would write to the output. This correctly used a lock to
prevent races, but there is an intermediate bytearray which is written
just beforehand (introduced with #1025) and this also needed to use
a lock to prevent races.

This change moves that intermediate bytearray under the same lock as
writing the output buffer, thus avoiding the race condition
@craigds craigds force-pushed the fix-garbled-jsonl-diff branch from f0d4d07 to e9b7a55 Compare February 6, 2025 22:07
@craigds craigds requested a review from olsen232 February 6, 2025 22:07
@craigds
Copy link
Member Author

craigds commented Feb 6, 2025

example of output

$ kart diff '42f601b^?...42f601b' -o json-lines --no-sort-keys --add-feature-count-estimate=exact
...
{"type":"feature","dataset":"nz_property_titles","change":{"+":{"fid":95,"geom":"01060000000100000001030000000100000009000000ACCE0CD2D8D66540F81CC19B6B6542C0378D3241DAD665408EEA74206B6542C050D6FE3EDAD66540D1A55AF06B6542C0E401D63ADAD6654036DEF8BF6C6542C0712BB834DAD66540DE10088F6D6542C0F520AE2CDAD66540E329645D6E6542C02C1725FDD9D665402732049B726542C0ACCE0CD2D8D6654045337118726542C0ACCE0CD2D8D66540F81CC19B6B6542C0","id":344  File "/opt/homebrew/Cellar/[email protected]/3.11.11/Frameworks/Python.framework/Versions/3.11/lib/python3.11/threading.py", line 982, in run
4244,"title_no":"NA36D/718","status":"LIVE","type":"Freehold","land_district":"North Auckland","issue_date":"1976-12-24T00:00:00.000","guarantee_status":"Guarantee","estate_description":"Fee Simple, 1/1, Lot 1 Deposited Plan 80161, 610 m2","number_owners":1,"spatial_extents_shared":"F"}}}
    self._target(*self._args, **self._kwargs)
  File "/Users/cdestigter/code/kart/kart/json_diff_writers.py", line 288, in _calculate_and_feature_count_estimate
    self.dump(
  File "/Users/cdestigter/code/kart/kart/json_diff_writers.py", line 252, in dump
    self._output_buffer.extend(b"\n")
BufferError: Existing exports of data: object cannot be re-sized
{"type":"featureCountEstimate","accuracy":"exact","datasets":{"nz_property_titles":98}}000010000000103000000010000000D0000004A65E5F2FAD66540E15A23C2636542C0730475B5F9D66540D0EF8C51566542C043131234F8D665400376DABF576542C05514AFB2F6D665405D42E02D596542C0FACA88F6F5D665405117F333516542C09BCC18A0F5D6654009C2A6694F6542C0A8E282CBF6D665402C43084D4E6542C0013B81F5FAD665402C43084D4E6542C0DE9A8A89FBD6654091282393546542C0E657B80BFAD66540777915FE556542C0658A3908FAD66540A231B801566542C00E1C8645FBD6654070A2DD72636542C04A65E5F2FAD66540E15A23C2636542C0","id":4410549,"title_no":"330406","status":"LIVE","type":"Cross lease","land_district":"North Auckland","issue_date":"2007-05-17T15:37:29.000","guarantee_status":"Guarantee","estate_description":"Fee Simple, 1/2, Lot 26 Deposited Plan 40676, 2,102 m2\r\nLeasehold, 1/1, Flat 1 Deposited Plan 382696","number_owners":2,"spatial_extents_shared":"T"}}}

@craigds craigds merged commit 31ef601 into master Feb 6, 2025
16 checks passed
@craigds craigds deleted the fix-garbled-jsonl-diff branch February 6, 2025 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants