brettch
commented
3 years ago Hmm, the logic does seem flawed. I think this was some of the first code written before I even decided to create Osmosis. I just wanted something faster than the existing Perl script that existed at the time. As a result it's pretty hacky, a class with over 1300 lines 😱
The most efficient approach would be to do a single commit at the end but for very large imports this will fail (I think, it's been a while). So there is a need to do the occasional interim commit.
Setting autoCommit to true will cause the database to do a commit after every single insert which is also very inefficient (okay, perhaps it's doing this anyway but it shouldn't be).
It looks to me like the dtCtx.commit() call at the end of each flushXXX method is in the wrong place. I think it should be inside the if(complete) clause instead of at the end of the method. If that was the case a commit would only occur after every 100,000 records as originally intended instead of after processing every single entity.
cvonsee
si-the-pie
It appears that the "flushxxxxx()" methods (
flushNodes(),flushNodeTags(),flushWays(), etc.) all issue a SQLcommit()at the end of the method, even when the logic to actually do SQL inserts of data is not executed.For example, in flushNodes() the
while (nodeBuffer.size() >= INSERT_BULK_ROW_COUNT_NODE)loop (starting around line 586) that inserts data into the DB is only executed every 100 times the method is called (100 being the value ofINSERT_BULK_ROW_COUNT_NODE), and yet thedbctx.commit()call is always executed at the end of the method. In addition, the 'complete' flag is false far more often than not and so the cleanup logic in that block is not executed and adbctx.commit()call is not needed.In addition, there is a commit near line 607 inside the
while (nodeBuffer.size() ...loop that is executed every 100K records (with 100,000 being the value ofTRANSACTION_SIZE:dbCtx.commit()simply checks to see if a DB connection is present (which appears to be always the case) and then issues the SQL commit() call.Redundant
dbctx.commit()calls are also made when sub-functions such asaddNodeTags()(which callsflushNodeTags()) are called, such as near line 616. Committing of node tag inserts (or way tag inserts, or whatever) can be done in the same commit that handles the node or way as long as it's done on the same database connection.I would propose that we either conditionally execute
dbctx.commit()only if inserts have actually been executed, or just movedbctx.commit()calls to just outside the logic that actually does inserts. When the DB connection is created 'autoCommit' is set to 'false', so since we're doing batch inserts it's conceivably an option to just turn autocommit on and let the database handle this.Updating these methods to restructure the logic and only issue
dbctx.commit()requests when actually needed would likely speed up database insert processing by a non-trivial amount.