Skip to content

AK to CCS 2.5 Merge - #432

Merged
Andrew Egelhofer (andrewegel) merged 16 commits into
confluentinc:2.5from
andrewegel:2.5
Oct 20, 2020
Merged

AK to CCS 2.5 Merge#432
Andrew Egelhofer (andrewegel) merged 16 commits into
confluentinc:2.5from
andrewegel:2.5

Conversation

@andrewegel

@andrewegel Andrew Egelhofer (andrewegel) commented Oct 20, 2020

Copy link
Copy Markdown

This was a merge from AK/2.5 -> CCS/2.5 - Merge was mostly clean except for Jenkinsfile (which I just discarded and took CCS's Jenkinsfile), and in build.gradle around some conditionals, which ammounts to this diff:

diff --cc build.gradle
index c504ddadf1,c7bb31795a..d6d0d37895
--- a/build.gradle
+++ b/build.gradle
@@@ -473,35 -450,29 +482,35 @@@ subprojects
        "-Xlint:poly-implicit-overload",
        "-Xlint:private-shadow",
        "-Xlint:stars-align",
 -      "-Xlint:type-parameter-shadow",
 -      "-Xlint:unused"
 +      "-Xlint:type-parameter-shadow"
      ]

 -    // Inline more aggressively when compiling the `core` jar since it's not meant to be used as a library.
 -    // More specifically, inline classes from the Scala library so that we can inline methods like `Option.exists`
 -    // and avoid lambda allocations. This is only safe if the Scala library version is the same at compile time
 -    // and runtime. We cannot guarantee this for libraries like kafka streams, so only inline classes from the
 -    // Kafka project in that case.
 -    List<String> inlineFrom
 -    if (project.name.equals('core'))
 -      inlineFrom = ["-opt-inline-from:scala.**", "-opt-inline-from:kafka.**", "-opt-inline-from:org.apache.kafka.**"]
 -    else
 -      inlineFrom = ["-opt-inline-from:org.apache.kafka.**"]
 -
 -    // Somewhat confusingly, `-opt:l:inline` enables all optimizations. `inlineFrom` configures what can be inlined.
 -    // See https://www.lightbend.com/blog/scala-inliner-optimizer for more information about the optimizer.
 -    scalaCompileOptions.additionalParameters += ["-opt:l:inline"]
 -    scalaCompileOptions.additionalParameters += inlineFrom
 +    if (versions.baseScala != '2.11') {
 +      scalaCompileOptions.additionalParameters += [
 +        "-Xlint:constant",
 +        "-Xlint:unused"
 +      ]

 +      // Inline more aggressively when compiling the `core` jar since it's not meant to be used as a library.
 +      // More specifically, inline classes from the Scala library so that we can inline methods like `Option.exists`
 +      // and avoid lambda allocations. This is only safe if the Scala library version is the same at compile time
 +      // and runtime. We cannot guarantee this for libraries like kafka streams, so only inline classes from the
 +      // Kafka project in that case.
 +      List<String> inlineFrom
 +      if (project.name.equals('core'))
 +        inlineFrom = ["-opt-inline-from:scala.**", "-opt-inline-from:kafka.**", "-opt-inline-from:org.apache.kafka.**"]
 +      else
 +        inlineFrom = ["-opt-inline-from:org.apache.kafka.**"]
 +
 +      // Somewhat confusingly, `-opt:l:inline` enables all optimizations. `inlineFrom` configures what can be inlined.
 +      // See https://www.lightbend.com/blog/scala-inliner-optimizer for more information about the optimizer.
 +      scalaCompileOptions.additionalParameters += ["-opt:l:inline"]
 +      scalaCompileOptions.additionalParameters += inlineFrom
 +    }
 +
-   // these options are valid for Scala versions < 2.13 only
-   // Scala 2.13 removes them, see https://github.com/scala/scala/pull/6502 and https://github.com/scala/scala/pull/5969
+     // these options are valid for Scala versions < 2.13 only
+     // Scala 2.13 removes them, see https://github.com/scala/scala/pull/6502 and https://github.com/scala/scala/pull/5969
 -    if (versions.baseScala == '2.12') {
 +    if (versions.baseScala in ['2.11','2.12']) {
        scalaCompileOptions.additionalParameters += [
          "-Xlint:by-name-right-associative",
          "-Xlint:unsound-match"

I additionally cherry-picked apache@63f3e1c test fix core/src/test/scala/unit/kafka/coordinator/group/GroupMetadataManagerTest.scala into this PR as that fixes a test-in-scala-2.11. I'll leave it up to the @confluentinc/kafka-eng team to decide whether this should be put into AK, and then merge over from there, or make this a CCS-only fix.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Luke Chen (showuon) and others added 14 commits September 21, 2020 13:42
… coordinator (apache#9202)

Fix the `currentStateTimeStamp` doesn't get set in `GROUP_METADATA_VALUE_SCHEMA_V3`, and did a small refactor to use the `GROUP_VALUE_SCHEMAS.size - 1` replace the default hard-coded max version number. Also add test for it.

Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk>
…pache#8181)

Avoid forwarding a shared reference to the record context in punctuate calls.
Note, this fix isn't airtight, since all processors triggered by a single punctuate
call will still see the same reference to the record context. It's also not a terribly
principled approach, since the context is still technically not defined, but this
is about the best we can do without significant refactoring. We will probably
follow up with a more comprehensive solution, but this should avoid the issue
for most programs.

Reviewers: Matthias J. Sax <mjsax@apache.org>, John Roesler <vvcephei@apache.org>
Adds a new task (`:streams:testAll`) to gradle to run all the tests for all Streams sub-projects.

Reviewers: Boyang Chen <boyang@confluent.io>
…tchup fails once (apache#8973)

Add logic to reset the existing `canReadConfigs` in `DistributedHerder` once the herder is able to successfully read the configs again. Added unit test to verify the functionality.

Author: Chris Egerton <chrise@confluent.io>
Reviewer: Nigel Liang <nigel@nigelliang.com>, Randall Hauch <rhauch@gmail.com>
…ULL nodes (apache#9306)

Fixes a regression introduced in `JsonConverter` with previous upgrades from Jackson Databind 2.9.x to 2.10.x. Jackson Databind version 2.10.0 included a backward-incompatible behavioral change to use `JsonNodeType.MISSING` (and `MissingNode`, the subclass of `JsonNode` that has a type of `MISSING`) instead of `JsonNodeType.NULL` / `NullNode`. See FasterXML/jackson-databind#2211 for details of this change.

This change makes recovers the older `JsonConverter` behavior of returning null on empty input.

Added two unit tests for this change. Both unit tests were independently tested with earlier released versions and passed on all versions that used Jackson 2.9.x and earlier, and failed on all versions that used 2.10.x and that did not have the fixed included in the PR. Both of the new unit tests pass with this fix to `JsonConverter`.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
)

System.currentTimeMillis() is not monotonic, so using that to calculate time to sleep can result in negative values. That will throw IllegalArgumentException.

This change checks for that and sleeps for a second (to avoid tight loop) if the value returned is negative.

Author: Shaik Zakir Hussain <zhussain@confluent.io>
Reviewer: Randall Hauch <rhauch@gmail.com>
Reviewers: Matthias J. Sax <matthias@confluent.io>
…o scale. (apache#9320)

The `org.apache.kafka.connect.data.Values#parse` method parses integers, which are larger than `Long.MAX_VALUE` as `double` with `Schema.FLOAT64_SCHEMA`.

That means we are losing precision for these larger integers.

For example:
`SchemaAndValue schemaAndValue = Values.parseString("9223372036854775808");`
returns:
`SchemaAndValue{schema=Schema{FLOAT64}, value=9.223372036854776E18}`

Also, this method parses values that can be parsed as `FLOAT32` to `FLOAT64`.

This PR changes parsing logic, to use `FLOAT32`/`FLOAT64` for numbers that don't have fraction part(`decimal.scale()!=0`) only, and use an arbitrary-precision `org.apache.kafka.connect.data.Decimal` otherwise.
Also, it updates the method to parse numbers, that can be represented as `float` to `FLOAT64`.

Added unit tests, that cover parsing `BigInteger`, `Byte`, `Short`, `Integer`, `Long`, `Float`, `Double` types.

Reviewers: Konstantine Karantasis <k.karantasis@gmail.com>
Fix infinite loop in Values::parseString

Author: Chris Egerton <chrise@confluent.io>
Reviewers: Konstantine Karantasis <konstantine@confluent.io>, Randall Hauch <rhauch@gmail.com>
…ues for properties not in connector’s ConfigDef (apache#9425)

Connect should not always add an error to configuration values in validation results that don't have a `ConfigKey` defined in the connector's `ConfigDef`, and any errors on such configuration values included by the connector should be counted in the total number of errors. Added more unit tests for `AbstractHerder.generateResult(...)`.

Author: Randall Hauch <rhauch@gmail.com>
Reviewer: Konstantine Karantasis <konstantine@confluent.io>
Trigger task reconfiguration when:
- topic-partitions are created or deleted on source cluster
- topic-partitions are missing on target cluster

Authors: Mickael Maison <mickael.maison@gmail.com>, Edoardo Comar <ecomar@uk.ibm.com>
Reviewer: Randall Hauch <rhauch@gmail.com>
…he#9305)

ClusterConnectStatesTest and ClientUtilsTest were failing because they expected
kafka.apache.org to resolve to 2 IP addresses. This updates the tests so they reflect
that DNS resolves to 3 addresses.

Reviewers: Ismael Juma <ismael@juma.me.uk>
This was a cherry-pick for the fix in core/src/test/scala/unit/kafka/coordinator/group/GroupMetadataManagerTest.scala
from 63f3e1c

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Andrew Egelhofer (@andrewegel) Thanks for the PR. LGTM.

@andrewegel
Andrew Egelhofer (andrewegel) merged commit fbe4053 into confluentinc:2.5 Oct 20, 2020
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.