Skip to content

Fix reference to INDEX_MAKER in IndexGeneratorJob.#1925

Merged
gianm merged 1 commit intoapache:masterfrom
gianm:fix-index-generator
Nov 6, 2015
Merged

Fix reference to INDEX_MAKER in IndexGeneratorJob.#1925
gianm merged 1 commit intoapache:masterfrom
gianm:fix-index-generator

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Nov 6, 2015

Fix reference missed in #1895

@drcrallen
Copy link
Copy Markdown
Contributor

I don't think that's a "fix" so much as it is a "change hadoop to use IndexMerger"

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Nov 6, 2015

@drcrallen this method was using IndexMerger before #1895, which unintentionally changed it to use IndexMaker

@drcrallen
Copy link
Copy Markdown
Contributor

nooo.... that PR moved the persist method above it from maker to merger. you are now also moving the merge method from maker to merger.

@drcrallen
Copy link
Copy Markdown
Contributor

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Nov 6, 2015

@drcrallen the intent of that PR was to remove LegacyIndexGeneratorJob and make IndexGeneratorJob behave like LegacyIndexGeneratorJob did. see also this if statement that was removed:

https://github.com/druid-io/druid/pull/1895/files#diff-0c9aca6f2536c742356ddc0bae3df945L68

"isPersistInHeap" would always be false (because it wasn't documented) so LegacyIndexGeneratorJob would always have got used. So #1895 was trying to collapse those without changing any behavior, by adjusting IndexGeneratorJob to behave like LegacyIndexGeneratorJob did. But this method got missed.

@himanshug
Copy link
Copy Markdown
Contributor

👍

@drcrallen
Copy link
Copy Markdown
Contributor

@gianm much better explanation, thanks!

@drcrallen
Copy link
Copy Markdown
Contributor

👍 after travis

gianm added a commit that referenced this pull request Nov 6, 2015
Fix reference to INDEX_MAKER in IndexGeneratorJob.
@gianm gianm merged commit dfbd0e2 into apache:master Nov 6, 2015
@gianm gianm deleted the fix-index-generator branch November 6, 2015 17:56
@gianm gianm added this to the 0.9.0 milestone Nov 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants