Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix web impl streaming line breaks with large rows #333

Merged
merged 7 commits into from
Oct 5, 2024

Conversation

slvrtrn
Copy link
Contributor

@slvrtrn slvrtrn commented Sep 30, 2024

Summary

Resolves #205
See also: #171, #204 (there was the same fix, but for Node.js impl only)

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

@slvrtrn slvrtrn requested a review from mshustov September 30, 2024 21:07
@slvrtrn
Copy link
Contributor Author

slvrtrn commented Sep 30, 2024

SUMMARY:
✔ 562 tests completed
ℹ 4 tests skipped
✖ 4 tests failed

FAILED TESTS:
  [Web] SELECT streaming
    should correctly process multiple chunks
      large amount of rows
        ✖ should work with .json()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8252:43)

        ✖ should work with .stream()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8265:43)

      rows that don't fit into a single chunk
        ✖ should work with .json()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8281:43)

        ✖ should work with .stream()
          Chrome Headless 129.0.0.0 (Linux x86_64)
        TypeError: Failed to fetch
            at WebConnection.request (webpack/commons.js:8674:36)
            at WebConnection.insert (webpack/commons.js:8606:37)
            at WebClickHouseClientImpl.insert (webpack/commons.js:6159:46)
            at genLargeStringsDataset (webpack/commons.js:5526:18)
            at async UserContext.<anonymous> (webpack/commons.js:8294:43)

Apparently, POST with a very large body size does not work well with Chrome Runner :/
For now, it might be better to generate the test data on the CH side.

EDIT: this is due to https://fetch.spec.whatwg.org/#http-network-or-cache-fetch

If contentLength is non-null and httpRequest’s keepalive is true, then:
If the sum of contentLength and inflightKeepaliveBytes is greater than 64 kibibytes, then return a network error.

Disabling keep-alive resolves the test failure.

Copy link

sonarqubecloud bot commented Oct 5, 2024

@slvrtrn slvrtrn merged commit 1c9b28b into main Oct 5, 2024
27 checks passed
@slvrtrn slvrtrn deleted the fix-web-streaming-line-breaks branch October 5, 2024 14:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Streaming line breaks in web version
2 participants