Update xmlelementreader.py #66
Conversation
when using self.root.clear() not all objects are imported into the database. Use elem.clear() instead.
|
Bedankt voor de patch, Vincent. Ik zal hem grondig testen. Zoals je in de history kunt zien, hebben we verschillende dingen geprobeerd om het geheugengebruik in te dammen of crashes te voorkomen.
Afgelopen week heb ik ook de andere schaalniveaus van de BRT toegevoegd. Ook daar treedt het issue op :( Voordeel, het testen gaat veel sneller met de Top500 dan de Top10. ;)
Verstuurd vanaf mijn iPhone
… Op 19 jan. 2018 om 13:11 heeft Vincent Nuhaan ***@***.***> het volgende geschreven:
when using self.root.clear() not all objects are imported into the database. Use elem.clear() instead.
You can view, comment on, or merge this pull request online at:
#66
Commit Summary
Update xmlelementreader.py
File Changes
M stetl/filters/xmlelementreader.py (2)
Patch Links:
https://github.com/geopython/stetl/pull/66.patch
https://github.com/geopython/stetl/pull/66.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Vincent bedankt! @fsteggink wil je eerst deze PR mergen en dan testen? Kan je zelf ook denk ik. |
|
Vanmiddag heb ik een test gedaan met de TOP500NL. Hier had ik zo'n 0,3% meer wegdelen. De fix werkt an sich wel. De TOP50NL loopt nog en de rest heb ik nog niet nader bekeken. Dat ga ik morgen doen. Zojuist heb ik op de NLExtract server de verwerking van de TOP10NL van november gestart, o.b.v. het filechunk bestand. Vannacht is dit klaar. Een vrijwilliger mag hier morgen wel naar kijken :). Ik denk niet dat ik er zelf aan toekom, i.i.g. niet overdag. 's Avonds kan ik wel een kleine check doen o.b.v. aantallen. Morgenavond ga ik de BGT met de fix testen, met name of het geheugenissue dat eerder optrad niet meer voorkomt. V.w.b. aantallen geloof ik het wel. Misschien ga ik nog wel een vergelijking doen o.b.v. actueelbestaand. Een vergelijking bij vervallen objecten heeft geen zin, omdat ik besloten heb een ontdubbelscript uit te schakelen (ivm een issue dat in de BGT-export zit), vanwege de lange doorlooptijd. |
|
Ik heb in 1e instantie voor self.root.clear gekozen, om te voorkomen dat het rootelement een hele serie lege child objecten zal bevatten, zoals gemeld in de link in het Python-commentaar. @VNuhaan Zou jij in je patch het commentaar hieromtrent bijwerken en evt. toelichten waarom je voor elem.clear kiest? Een paar zinnen is voldoende, geen uitgebreid verhaal ;) |
|
Ik weet niet hoe ik commentaar achteraf kan toevoegen, maar in de link staat een voorbeeld. Daarin staat dat het xml element elem heet (event, elem =..). Alle voorbeelden die ik heb gezien werkten op element niveau en niet op root niveau. Daarbij heb ik mijn twijfels of het end event alleen bij een afsluitende tag wordt afgevuurd. Ik vermoed dat deze ook af wordt gevuurd bij het einde van een blok data dat geanalyseerd wordt. |
|
Dat zou kunnen. Zoiets vermoed ik ook. Event driven verwerkingen zijn altijd lastig te debuggen, omdat er van alles en nog wat gebeurt, dus je raakt gauw het overzicht kwijt. Zeker als bugs getriggerd worden bij grote hoeveelheden data. Vannacht heb ik de TOP10NL met Stetl geladen. De dump is, voor zover ik kan zien, compleet. De BGT is nu nog bezig. Al ongeveer 12 uur, dat zijn tijden om je voor te schamen, alhoewel ik heb gehoord dat het bij andere softwareproducten dagen kan duren ;) Het is i.i.g. een goed teken dat hij nog laadt, dus het geheugenissue wat we eerst hadden, lijkt niet meer zijn kop op te steken. Als die weer terug zou zijn, zou ik dat heel gek hebben gevonden, omdat de meeste data nu wel wordt verwijderd, alleen een loze entry in het root document niet. Lijkt me een goede trade-off. Voor de iets langere termijn wil ik de verwerking veel simpeler maken, mede o.b.v. de ideeën die jij hebt aangedragen, maar er kunnen situaties zijn dat Stetl wel voor het verwerken en opknippen van XML wordt gebruikt, dus het is wel goed dat het nu voor 99% opgelost is :) |
|
@justb4 Ik heb geen merge-knop hier. Wijzignen aan Stetl dien ik zelf ook altijd via een PR in. |
|
Ok, gemerged. Kan bij missende objecten ook niet #49 nog opspelen: bijv |
|
Vwb het commentaar, ik heb hiervoor PR #67 ingediend. |
when using self.root.clear() not all objects are imported into the database. Use elem.clear() instead.