Skip to content

Delete buildV9Directly in Kafka and Kinesis Indexing Service#11351

Merged
clintropolis merged 5 commits intoapache:masterfrom
bananaaggle:delete_buildV9Directly_in_kafka_and_kinesis_indexing_service
Jun 23, 2021
Merged

Delete buildV9Directly in Kafka and Kinesis Indexing Service#11351
clintropolis merged 5 commits intoapache:masterfrom
bananaaggle:delete_buildV9Directly_in_kafka_and_kinesis_indexing_service

Conversation

@bananaaggle
Copy link
Copy Markdown
Contributor

Parameter "buildV9Directly" in Kafka and Kinesis tunningConfig will be removed 0.12, but it still reserved in code. I think it is safe to remove it.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@bananaaggle bananaaggle changed the title delete_buildV9Directly_in_kafka_and_kinesis_indexing_service Delete buildV9Directly in Kafka and Kinesis Indexing Service Jun 9, 2021
@FrankChen021
Copy link
Copy Markdown
Member

FrankChen021 commented Jun 9, 2021

Thanks for cleaning up the code.

I see this parameter is still referenced in 'native-batch.md' but not used in the ParallelIndexTuningConfig. Could you delete it from the doc ?

@bananaaggle
Copy link
Copy Markdown
Contributor Author

Thanks for cleaning up the code.

I see this parameter is still referenced in 'native-batch.md' but not used in the ParallelIndexTuningConfig. Could you delete it from the doc ?

I find it, but I'm not very sure delete this parameter from batch job is safe.

@FrankChen021
Copy link
Copy Markdown
Member

I find it, but I'm not very sure delete this parameter from batch job is safe.

Actually it's not used in the code, you could check it.

@bananaaggle
Copy link
Copy Markdown
Contributor Author

I find it, but I'm not very sure delete this parameter from batch job is safe.

Actually it's not used in the code, you could check it.

I delete this method from every where I can find it and pass all unit tests. This is a field in restful API, so I think it's safe to remove it even if some old config had it, because it will not be used and cause no exception.

@FrankChen021
Copy link
Copy Markdown
Member

Pls check the CI.

@bananaaggle
Copy link
Copy Markdown
Contributor Author

Pls check the CI.

Thanks! Checking now.

@bananaaggle
Copy link
Copy Markdown
Contributor Author

Hi, @FrankChen021 , happy weekend! I run some integration tests on one EC2 like -Dgroups=batch-index, -Dgroups=query-retry, this branch passed those tests. But it failed on Travis. Is there something wrong for integration tests in Travis?

@FrankChen021
Copy link
Copy Markdown
Member

Hi, @FrankChen021 , happy weekend! I run some integration tests on one EC2 like -Dgroups=batch-index, -Dgroups=query-retry, this branch passed those tests. But it failed on Travis. Is there something wrong for integration tests in Travis?

Might be due to flaky test. I re-triggered the failed tasks. Let's wait to see the result.

@bananaaggle
Copy link
Copy Markdown
Contributor Author

Hi, @FrankChen021 , happy weekend! I run some integration tests on one EC2 like -Dgroups=batch-index, -Dgroups=query-retry, this branch passed those tests. But it failed on Travis. Is there something wrong for integration tests in Travis?

Might be due to flaky test. I re-triggered the failed tasks. Let's wait to see the result.

Thank you very much!

Copy link
Copy Markdown
Member

@FrankChen021 FrankChen021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@suneet-s
Copy link
Copy Markdown
Contributor

Added release notes, just in case someone is still using this even though it was deprecated for a very long time

If the release manager thinks we don't need to call this out - we can exclude this from the release notes

@clintropolis
Copy link
Copy Markdown
Member

Added release notes, just in case someone is still using this even though it was deprecated for a very long time

This isn't necessary, it looks like this was left in place for upgrades between 0.11 and 0.12, and we just forgot to remove it. It's not hooked into anything, since all of that stuff was removed in #4420.

@clintropolis clintropolis merged commit de8daf8 into apache:master Jun 23, 2021
jihoonson pushed a commit to jihoonson/druid that referenced this pull request Jul 12, 2021
…11351)

* delete_buildV9Directly_in_kafka_and_kinesis_indexing_service

* delete

* delete them from server

* delete buildV9Directly from hadoop indexing

* bug fixed

Co-authored-by: yuanyi <yuanyi@freewheel.tv>
@clintropolis clintropolis added this to the 0.22.0 milestone Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants