keboola / php-db-import

MIT License
4 stars 0 forks source link

Snowflake - support more than 1000 slices #39

Closed Halama closed 6 years ago

Halama commented 6 years ago

Aktuálně používáme v COPY commandu property FILES která má limit 1000. Pokud má soubor více slices vracíme user error.

https://github.com/keboola/php-db-import/blob/3606e32659978b5a5151b8484fb1ec072cdb0071/src/Snowflake/CsvImportBase.php#L109

Halama commented 6 years ago

https://docs.snowflake.net/manuals/sql-reference/sql/copy-into-table.html

Halama commented 6 years ago

Možná můžeme např. jenom rozsekat po tisících a volat COPY vícekrát po sobě.

pivnicek commented 6 years ago

Jo, vypada ze je to asi hard limit

image

Halama commented 6 years ago

yes it is

Halama commented 6 years ago

Trochu jsem se na to dival a klonil bych se asi k tomu to rozsekat prostě po 1000 filech a pro každou dávku pak volal samostatný COPY command. Vždy se importuje do staging tabulky takže nehrozí nekonzistence v případě importu jedné a failu druhé.

Další možností by bylo použít PATTERN místo FILES kvůli vynechání manifest souboru, je potřeba ale zajistit at to nevynechá nic jiného.

ErikZigo commented 6 years ago

Tohle by se melo vyresit i v KBC. Kde failne restore velke tabulky ze snapshotu https://github.com/keboola/connection/issues/1031

Halama commented 6 years ago

jo resim to kvuli kbc, tam se tohle pouziva pro importy i restore.

Halama commented 6 years ago

i z toho duvodu tech snapshotu by teda bylo dobry to fixnout, protoze snapshot ktery nejde restornout je trochu k nicemu :) I kdyz ted s releasem timz e travel snapshoty nebudou zas tolik potreba, resp. budou potreba jenom pokud si neco budes chtit odlit dlouhodobeji nez je retence projektu.

ErikZigo commented 6 years ago

Jo, nejak to issue v KBC vyhnilo :-)

Halama commented 6 years ago

Až to fixneme tak bude potřeba odstranit zmínku tady https://developers.keboola.com/extend/common-interface/folders/#sliced-tables

ondrejhlavacek commented 6 years ago

+1

taky mě napadlo to nadávkovat po 1000, tím, že je to do stagingu, tak by to nemělo ničemu vadit.

ondrejhlavacek commented 6 years ago

https://keboola.slack.com/archives/C02C3GZUS/p1524212866000219 https://keboola.zendesk.com/agent/tickets/8292

Halama commented 6 years ago

On by asi ani ten PATTERN neměl být problém. Jenom bych to nedělal takhle https://github.com/keboola/transformation-bundle/blob/c474afec5a47a633f35b6cd4e9409088a21f144d/library/Keboola/Transformation/Backend/Snowflake.php#L348 protože v nějaké bizar situaci by tam klidně mohlo být více souborů které končí manifest.

Pokud by se ten regexp napsal tak ať to vynechá $fileInfo["s3Path"]["key"].manifest tak by to mělo být bezpečný.

Halama commented 6 years ago

I když vlastně ne. Teď říkáme že to importuje soubory které jsou zmíněné v manifestu. Ignorovat ho by mohl být BC break!

Halama commented 6 years ago

Problém může být s tím že potom ten JSON manifest může být velký https://github.com/keboola/connection/issues/867

Halama commented 6 years ago

Řešením by potom mohlo být deprekování manifestů a importovat prostě vše co leží pod daným prefixem. To ale může být až další krok.

ondrejhlavacek commented 6 years ago

Co kdyby se teda udělal druhej import, kterej by byl bez manifestu?

Mezitím jsem tady teda pracně vypotil regulár, kterej matchne všechny cesty kromě manifestu :-)

https://regex101.com/r/lTyF5h/1/

Halama commented 6 years ago

Tam hrozí ten BC break. To by se muselo nějak kontrolovaně zavést jako typ importu/souboru v SAPI.

ondrejhlavacek commented 6 years ago

No vždyť to jsem psal :-)

Halama commented 6 years ago

Jako že rovnou připravovat variantu bez manifestů?

ondrejhlavacek commented 6 years ago

Jj, než to nějak pytlíkovat po tisícovkách (tam to taky může na něco narazit?).

ondrejhlavacek commented 6 years ago

Ten manifest tam byl jen pro Redshift, ne?

Halama commented 6 years ago

Vzniklo to s Resdhiftem

Halama commented 6 years ago

Tak já na to založím asi issue v KBC

ondrejhlavacek commented 6 years ago

Bude to chtít ověřit, že to zvládne velký tisíce souborů, chceš s tím nějak pomoct?

Halama commented 6 years ago

Toho se nebojím, spíš bude potřeba pořádně promyslet, kompatibilitu, čeho všeho se to bude týkat atd.

Halama commented 6 years ago

Dovedu si představit že bude asi poměrně jednoduché zavést import kterému se řekne parametrem ať ignoruje manifest. Jak se ale kompletně zbavit manifestů, tzn. aby mohli existovat soubory bez nich zatím moc nevím. Všechny aktuální verze sapi klienta a další integrace by přestaly fungovat.

ondrejhlavacek commented 6 years ago

Pro Redshift ho stejně furt potřebujem.

Halama commented 6 years ago

Ono to vypadá že ani Redshift to nevyžaduje https://docs.aws.amazon.com/redshift/latest/dg/loading-data-files-using-manifest.html

ondrejhlavacek commented 6 years ago

Tak to by bylo super!

Halama commented 6 years ago

Jenže Redshift zas neumí ten PATTERN který je potřeba aby to ignorovalo ten manifest:( https://docs.aws.amazon.com/redshift/latest/dg/copy-parameters-data-source-s3.html

Halama commented 6 years ago

Začal jsem to sepisovat a vlastně moc nevím jak z toho ven.

Halama commented 6 years ago

U Redshiftu ale zas není tohle issue s velikostí nebo počtem slices, takže tam by se to mohlo chovat pořád stejně. I když je blbost mít parametr import ignoreManifest a ve skutečnosti ho používat pro RS.

ondrejhlavacek commented 6 years ago

No tak by se ty data pro import připravily bez manifestu. Může to bejt parametr na API nebo novej API call, kterej nevyžaduje manifest.

Halama commented 6 years ago

To by možná šlo, tzn. byla by to feature files a ne importu. Import by se podle toho pak jenom zachoval a switchnul na jednu z verzí. File by měl nově flag hasManifest.

Tohodle řešení jsem se bál kvůli BC. Jde o to že ze stažením takového souboru by si neporadili aktuální verze SAPI klientů. Pokud by to začal nastavovat docker runner pro soubory které importuje to tabulek tak tam by asi problém nebyl. Nenapadá mě situace kdy by se soubor který byl jednou importovaný dále nějak zpracovával.

Otázka je co s tím pak dál pokud bysme chtěli kompletně přejít na files bez manifestů?

Halama commented 6 years ago

Shodou okolností se v pátek objevilo jiné issue s manifesty:) https://github.com/keboola/connection/issues/332#issuecomment-383366066

ondrejhlavacek commented 6 years ago

Jo aha, v tomhle je fígl, to mě nenapadlo. Bylo by super, kdyby starej SAPI klient dokázal říct, že ten soubor neumí stáhnout, ať použiješ novější. To by se tam mohlo nějak protlačit z API přinejhorším jako chyba na API?

Halama commented 6 years ago

No ale tím by to mohlo někomu rozbít integraci, taky vůbec nemusí používat naše klienty. Na naší straně bysme to ale poupgradovali v pohodě. Pak bysme viděli co zbylo.

Ještě přemýšlím jestli je nějaký důvod ty manifesty zachovat. Jediné co mě napadá je konzistence listování souborů v S3, to znovu ověřím.

ondrejhlavacek commented 6 years ago

Jo, to mohlo :-( Co myslíš tou konzistencí listování souborů v S3?

Halama commented 6 years ago

Napadlo mě že bysme tam ten manifest prostě pro BC mohli nějak dogenerovávat. Blbý je že se přesně nedá určit okamžik kdy se to má stát:(

Halama commented 6 years ago

Tou konzistencí myslím jako že tam nasypeš najednou x souborů. A ihned potom zavoláš ten import který zavolá list nad tím prefixem v S3, tak jestli se nemůže stát že některý se tam třeba hned neobjeví.

ondrejhlavacek commented 6 years ago

Fabuluju - co kdyby měl manifest a soubory unikátní prefix? Potom by sis mohl šáhnout na manifest, kterej je mimo prefix/namespace souborů. Příklad

s3-bucket/folder/myfileimport/manifest
s3-bucket/folder/myfileimport/files/part0
s3-bucket/folder/myfileimport/files/part1
s3-bucket/folder/myfileimport/files/part2

Má to společnej prefix, v manifestu budou cesty na soubory, a přitom můžeš mít i prefix, kterej je jen na soubory.

Halama commented 6 years ago

jo to že leží na stejné urovni jako slices je taky jeden z problémů. Jenže o to kam se co nahrává se opět starají klienti. Tzn. pokud by se to měnilo museli bysme to říct. Takže to v tuhle chvíli asi moc nepomůže. A spíš bych postupoval k tomu odstranění manifestu.

ondrejhlavacek commented 6 years ago

Nojo vlastně. Nemůžu dohledat, jestli má Redshift nějaký omezení na počet souborů v manifestu pro import. Pokud nic takovýho nemá, tak by se na Redshift moh použít manifest a na Snowflake ho pomocí regexpu vyignorovat?

Halama commented 6 years ago

To se ale zas vracíme k tomuhle https://github.com/keboola/php-db-import/issues/39#issuecomment-383028753 To už bych raděi udělal ty loady ve smyččce po 1000, to asi můžeme udělat rovnou. To by byl quickfix ale myslím že bysme se tech manifestů měli zbavit i kvůli dalším issues.

ondrejhlavacek commented 6 years ago

Nestačilo by k tomu https://github.com/keboola/php-db-import/issues/39#issuecomment-383028753 prostě ohlásit, že od nějaký chvíle se pro Snowflake načtou všechny soubory? Bude teda vypadat divně, že to bude jen Snowlake a ne Redshift. Ale pro Redshift by to taky šlo obejít.

BC break to bude - ale Snowflake taky takovýhle věci pushuje ven a dá k tomu dopředu termín.

Halama commented 6 years ago

!! A process writes a new object to Amazon S3 and immediately lists keys within its bucket. Until the change is fully propagated, the object might not appear in the list.

Halama commented 6 years ago

https://docs.aws.amazon.com/AmazonS3/latest/dev/Introduction.html#ConsistencyModel

Halama commented 6 years ago

To imho znamená že manifesty musíme zachovat, protože to že to může skipnout nějaký slice je sakra nebezpečný.

Halama commented 6 years ago

Nebo se na to zeptat na supportu.