Skip to content

fix: Don't list Postgres tables during insert#13323

Merged
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:fix/dont_query_information_schema_postgres
Aug 25, 2023
Merged

fix: Don't list Postgres tables during insert#13323
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:fix/dont_query_information_schema_postgres

Conversation

@erezrokah
Copy link
Copy Markdown
Member

@erezrokah erezrokah commented Aug 25, 2023

Summary

Extracted from #13318. Witnessed the bottleneck using pprof.
With default batch settings I'm getting 2m29s sync time instead of ~11m,
With batch_size_bytes: 100000000 and batch_timeout: 60s I'm getting 1m40s. (Fixed in #13324)

Used spec:

kind: source
spec:
  name: aws
  path: "cloudquery/aws"
  version: "v19.0.0"
  tables:
    - "*"
  skip_tables:
    - "aws_cloudtrail_*"
    - "aws_iam_*"
    - "aws_servicequotas_*"
  destinations:
    - postgresql
---
kind: destination
spec:
  name: "postgresql"
  registry: "grpc"
  path: localhost:8888
  # path: cloudquery/postgresql
  # version "v5.0.5"
  spec:
    connection_string: "postgresql://postgres:pass@localhost:5432/postgres?sslmode=disable"
    # batch_size_bytes: 100000000
    # batch_timeout: "60s"

var sb strings.Builder
tName := table.Name
tableName := pgx.Identifier{tName}.Sanitize()
tableName := table.Name
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed these since I initially used tableName for the dictionary key which is wrong since it had the sanitized value

@erezrokah erezrokah requested a review from candiduslynx August 25, 2023 09:29
@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Aug 25, 2023
@kodiakhq kodiakhq bot merged commit 6dee8bc into cloudquery:main Aug 25, 2023
kodiakhq bot pushed a commit that referenced this pull request Aug 25, 2023
🤖 I have created a release *beep* *boop*
---


## [5.0.6](plugins-destination-postgresql-v5.0.5...plugins-destination-postgresql-v5.0.6) (2023-08-25)


### Bug Fixes

* **deps:** Update `github.com/cloudquery/plugin-sdk/v4` to v4.5.5 ([#13297](#13297)) ([38d4d59](38d4d59))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 5b83d4f ([#13203](#13203)) ([b0a4b8c](b0a4b8c))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.5.1 ([#13195](#13195)) ([a184c37](a184c37))
* Don't list Postgres tables during insert ([#13323](#13323)) ([6dee8bc](6dee8bc))
* Properly handle trailing zeroes in uint64 values when reading ([38d4d59](38d4d59))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
}
}

pkConstraintName := tableName + "_cqpk"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it actually use the c.pgTablesToPKConstraints value if that's available?

Copy link
Copy Markdown
Member Author

@erezrokah erezrokah Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside createTableIfNotExist so there should not be any value available

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, but what about table.PkConstraintName?

erezrokah added a commit that referenced this pull request Aug 25, 2023
…to v5.0.6" (#13326)

Reverts #13325

So I can fix #13323 when
`--no-migrate` is used
kodiakhq bot pushed a commit that referenced this pull request Aug 25, 2023

#### Summary

Follow up to #13323 to handle `--no-migrate`. Thanks @candiduslynx for pointing it out

<!--
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants