search

evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

Premature close mysql when building source #1389

Closed kamilglod closed 3 months ago

kamilglod commented 5 months ago

Steps To Reproduce

Steps we can take to reproduce the issue you're seeing. Sample data is helpful.

Environment

Expected Behavior

Build sources without error, or at least fail build - now it's silently skips that sources.

Actual Behaviour

I'm getting Premature close error when running evidence sources

# npm run build:sources

> XXX@0.0.1 build:sources
> evidence sources --debug

(node:109) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Processing prd_mysql_XXX
  XXX1 ✖ Premature close
  XXX2 ✖ Premature close
  XXX3 ✖ Premature close
  XXX4 ✖ Premature close
  XXX5 ✖ Parquet error: Incorrect number of rows, expected 16 != 12 rows
  XXX6 ✖ Premature close
  XXX7 ✖ Premature close
  XXX8 ✖ Premature close
  XXX9 ✖ Parquet error: Incorrect number of rows, expected 10 != 8 rows

Workarounds

Nope

ItsMeBrianD commented 5 months ago

👋 Thanks for reporting this!

Does the first row of your MySQL Source query include a null column? We've seen a similar issue in situations where that is the case

If not, can you provide a (anonymized) example of the table that you're loading?

kamilglod commented 5 months ago

@ItsMeBrianD Table:

mysql> CREATE TABLE test1 (
   id int NOT NULL AUTO_INCREMENT PRIMARY KEY,
   col1 varchar(255) NOT NULL,
   col2 varchar(255)
 );

Data: 

SELECT * FROM test1;
+----+------+------+
| id | col1 | col2 |
+----+------+------+
|  1 | a    | 1    |
|  2 | a    | 1    |
|  3 | a    | 1    |
|  4 | a    | 1    |
|  5 | a    | 1    |
|  6 | a    | 1    |
|  7 | a    | 1    |
|  8 | a    | 1    |
|  9 | a    | 1    |
| 10 | a    | 1    |
| 11 | a    | 1    |
| 12 | a    | 1    |
| 13 | a    | 1    |
| 14 | a    | 1    |
| 15 | a    | 1    |
| 16 | a    | 1    |
| 17 | a    | 1    |
| 18 | a    | 1    |
| 19 | a    | 1    |
| 20 | a    | 1    |
| 21 | a    | 1    |
+----+------+------+

Evidence query:

SELECT * FROM test1 LIMIT 17

docker-compose setup:

  mysql1:
    image: mysql:8.0
    restart: always
    environment:
      MYSQL_DATABASE: 'db'
      MYSQL_USER: 'user'
      MYSQL_PASSWORD: 'password'
      MYSQL_ROOT_PASSWORD: 'password'
    ports:
      - '3306:3306'

package.json

{
  "name": "XXX",
  "version": "0.0.1",
  "scripts": {
    "build": "evidence build",
    "build:strict": "evidence build:strict",
    "dev": "evidence dev --host 0.0.0.0",
    "test": "evidence build",
    "build:sources": "evidence sources"
  },
  "engines": {
    "npm": ">=7.0.0",
    "node": ">=16.14.0"
  },
  "type": "module",
  "dependencies": {
    "@evidence-dev/bigquery": "usql",
    "@evidence-dev/core-components": "usql",
    "@evidence-dev/csv": "usql",
    "@evidence-dev/evidence": "usql",
    "@evidence-dev/faker-datasource": "usql",
    "@evidence-dev/mysql": "usql",
    "@evidence-dev/postgres": "usql",
    "@evidence-dev/sqlite": "usql",
    "@evidence-dev/usql-datatable": "1.0.2",
    "duckdb": "^0.9.2",
    "mermaid": "^10.4.0"
  },
  "overrides": {
    "jsonwebtoken": "9.0.0",
    "trim@<0.0.3": ">0.0.3",
    "sqlite3": "5.1.5",
    "@evidence-dev/tailwind": "usql"
  }
}

Output:

root@2bd0a8a9f36e:/app# npm run build:sources

> ingestion-team-dev-docs@0.0.1 build:sources
> evidence sources

Processing foo
  foretag_market_agent_type ✖ Premature close

What is interesting is that LIMIT 17 is somehow magic number, when changing to 16 it's working correctly 🤷

ItsMeBrianD commented 5 months ago

What is interesting is that LIMIT 17 is somehow magic number, when changing to 16 it's working correctly 🤷

Yeah that sounds bizzare, thank you for the detailed repro steps, we'll take a look at this!

kamilglod commented 5 months ago

One side note that might worth mentioning is that when I'm running evidence sources I'm running into the situation where program hangs, regardless of result of the build. It's not quitting with any status code, I need to kill it in order to get back an access to the terminal.

kamilglod commented 4 months ago

I tested it with newest version after usql release, on new project generated by npx degit evidence-dev/template temporary, still failing with the same result.

mcrascal commented 4 months ago

Thanks Kamil, we'll take another look here.

aszenz commented 4 months ago

Faced the same issue, removing the emit close event worked temporarily

mcrascal commented 4 months ago

@csjh Check in other connectors as well for any that don't handle the async iterable

kamilglod commented 4 months ago

@ItsMeBrianD I tested the newest version of @evidence-dev/mysql@1.0.2 and @evidence-dev/evidence@24.0.8 and I'm still getting the same error:

Processing mysql_example
  active_shops ✖ Premature close
  all_shops_with_additional_info ✖ Premature close
  foretag_market_agent_type ✖ Premature close
  fr_active_shops ✖ Premature close
  fr_group_by_type_and_fail_status ✖ Premature close
  legacy_agents_delimiter ✖ Premature close
  legacy_agents_xml_node ✖ Premature close
  legacy_feeds_parser_info ✖ Premature close
  sample_shops ✔ Finished. 10 rows

it works for small data but having problem with bigger one (around 10k rows). I tested it in docker on Mac M2 but my colleague tested it too on Mac Intel with the same result.

I would love to provide you some more info but I'm not sure if evidence have any option of debug logs toggle while building sources.

mario-toursprung-com commented 3 months ago

Faced the same issue, removing the emit close event worked temporarily

Worked for me too. Commenting out line 282 in node_modules/mysql2/lib/commands/query.js // stream.emit('close'); // notify readers that query has completed

or replace the line with the following (see: https://github.com/sidorares/node-mysql2/commit/af4714845603f70e3c1ef635f6c0750ff1987a9e) setImmediate(() => stream.emit('close')); // notify readers that query has completed

e.g. sed -i "s/stream.emit('close');/setImmediate(() => stream.emit('close'));/g" node_modules/mysql2/lib/commands/query.js

Or update your node modules. Yesterday there was a new re lease of mysql2 addressing this very problem: https://github.com/sidorares/node-mysql2/releases/tag/v3.8.0

andrejohansson commented 3 months ago

@mario-toursprung-com thank you, I can confirm that adding 

    "mysql2": "3.8.0"

To the overrides section in package.json seem to work.