Fix imports of multiple databases in one export file #9352
Conversation
ghost
added
the
importer/v8/importer.go
Outdated
| @@ -178,10 +181,18 @@ func (i *Importer) processDML(scanner *bufio.Reader) error { | |||
| return nil | |||
| } | |||
| if strings.HasPrefix(line, "# CONTEXT-DATABASE:") { | |||
| i.database = strings.TrimSpace(strings.Split(line, ":")[1]) | |||
| i.dmlDatabase = strings.TrimSpace(strings.Split(line, ":")[1]) | |||
There was a problem hiding this comment.
Is there a reason why you don't just call i.batchWrite() here before changing the database? It seems like there would be less of a code change if the batch were just flushed whenever the context database or retention policy is changed. It looks like batchWrite() is pretty safe to call multiple times. The only caveat is that, for some reason, this section of code:
i.batch = i.batch[:0]This is in batchAccumulator rather than inside of batchWrite. If you move that piece of code to the end of batchWrite and remove it from batchAccumulator, you can just call batchWrite whenever you want to flush the writes.
There was a problem hiding this comment.
You're right, thanks. That was much easier. I rebased and pushed.
Not sure how to write a test with more than 5000 points without pasting in a gigantic string of json. Open to suggestions.
batchWrite was using the last database and retention policy read from the input file. Because batchWrite was called only every batchSize lines or at EOF, databases with fewer than batchWrite points could be imported into the incorrect database. This change forces a flush with batchWrite whenever processDML reads a change in database or retention policy.
|
I resolved the changelog conflict and pushed an update. As soon as it passes tests, I'm going to merge this. Thanks for the help! We should have this change included in the 1.5 release. |
|
Hi @wwilfinger, can you sign the link to the CLA that's here? Thanks. |
|
@wwilfinger ping. Can you sign the CLA otherwise we're going to have to revert this change and redo it. Thanks. |
|
@jsternberg My employer's legal team is taking a while to get back to me if I can sign the CLA. Go ahead and revert. I apologize for the waste of time. |
Fixes #9336
I'm not happy with some of the names here. I also want to add a test that hits the 5000 batchSize to verify everything works there. Will probably look at
cmd/influx_inspect/export/export_test.goto see how to generate larger test dbs.batchWrite was using the last database and retention policy read from
the input file. Because batchWrite was called only every batchSize lines
or at EOF, databases with fewer than batchWrite points could be imported
into the incorrect database.
This change forces a flush with batchWrite whenever processDML reads a
change in database or retention policy.
Required for all non-trivial PRs