You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by bs...@apache.org on 2020/03/12 19:42:11 UTC

[hive] branch master updated: HIVE-21218: add support topics created via Confluent Avro serializer (David McGinnis via Slim Bouguerra)

This is an automated email from the ASF dual-hosted git repository.

bslim pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git


The following commit(s) were added to refs/heads/master by this push:
     new 48bc9e3  HIVE-21218: add support topics created via Confluent Avro serializer (David McGinnis via Slim Bouguerra)
48bc9e3 is described below

commit 48bc9e389c9b88a6e25bec3fd19ed29130f3f156
Author: David McGinnis <mc...@davidmcginnis.net>
AuthorDate: Thu Mar 12 12:38:51 2020 -0700

    HIVE-21218: add support topics created via Confluent Avro serializer (David McGinnis via Slim Bouguerra)
---
 .gitignore                                         |   1 +
 kafka-handler/README.md                            |   3 +
 kafka-handler/pom.xml                              |  68 +++++++-
 .../org/apache/hadoop/hive/kafka/KafkaSerDe.java   |  71 ++++++++-
 .../hadoop/hive/kafka/AvroBytesConverterTest.java  | 172 +++++++++++++++++++++
 kafka-handler/src/test/resources/SimpleRecord.avsc |  13 ++
 .../hadoop/hive/serde2/avro/AvroSerdeUtils.java    |   2 +
 7 files changed, 321 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index 442c132..68ff2f6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,3 +37,4 @@ standalone-metastore/metastore-common/src/gen/version
 standalone-metastore/metastore-server/src/gen/version
 launch.json
 settings.json
+kafka-handler/src/test/gen
diff --git a/kafka-handler/README.md b/kafka-handler/README.md
index 753e3e3..e7761e3 100644
--- a/kafka-handler/README.md
+++ b/kafka-handler/README.md
@@ -50,6 +50,9 @@ ALTER TABLE
 SET TBLPROPERTIES (
   "kafka.serde.class" = "org.apache.hadoop.hive.serde2.avro.AvroSerDe");
 ```
+
+If you use Confluent's Avro serialzier or deserializer with the Confluent Schema Registry, you will need to remove five bytes from the beginning of each message. These five bytes represent [a magic byte and a four-byte schema ID from the registry.](https://docs.confluent.io/current/schema-registry/serializer-formatter.html#wire-format)
+This can be done by setting `"avro.serde.type"="skip"` and `"avro.serde.skip.bytes"="5"`. In this case it is also recommended to set the Avro schema either via `"avro.schema.url"="http://hostname/SimpleDocument.avsc"` or `"avro.schema.literal"="{"type" : "record","name" : "SimpleRecord","..."}`. If both properties are set then `avro.schema.literal` has higher priority.
  
 List of supported serializers and deserializers:
 
diff --git a/kafka-handler/pom.xml b/kafka-handler/pom.xml
index 6ad41de..4e58cb9 100644
--- a/kafka-handler/pom.xml
+++ b/kafka-handler/pom.xml
@@ -52,6 +52,11 @@
       </exclusions>
     </dependency>
     <dependency>
+      <groupId>org.apache.hive</groupId>
+      <artifactId>hive-serde</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
     </dependency>
@@ -60,9 +65,9 @@
       <artifactId>hadoop-common</artifactId>
       <exclusions>
         <exclusion>
-        <groupId>org.slf4j</groupId>
-        <artifactId>slf4j-api</artifactId>
-      </exclusion>
+          <groupId>org.slf4j</groupId>
+          <artifactId>slf4j-api</artifactId>
+        </exclusion>
       </exclusions>
     </dependency>
     <dependency>
@@ -80,6 +85,10 @@
       <artifactId>kafka-clients</artifactId>
       <version>${kafka.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.apache.avro</groupId>
+      <artifactId>avro</artifactId>
+    </dependency>
     <!-- test pkg-->
     <dependency>
       <groupId>junit</groupId>
@@ -118,8 +127,27 @@
       <version>1.7.30</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>io.confluent</groupId>
+      <artifactId>kafka-avro-serializer</artifactId>
+      <version>5.4.0</version>
+      <scope>test</scope>
+      <exclusions>
+        <exclusion>
+          <groupId>org.apache.avro</groupId>
+          <artifactId>avro</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
   </dependencies>
 
+  <repositories>
+    <repository>
+      <id>confluent</id>
+      <url>http://packages.confluent.io/maven/</url>
+    </repository>
+  </repositories>
+
   <profiles>
     <profile>
       <id>dev-fast-build</id>
@@ -179,16 +207,44 @@
     <testSourceDirectory>${basedir}/src/test</testSourceDirectory>
     <plugins>
       <plugin>
-        <groupId>org.apache.maven.plugins</groupId>
-        <artifactId>maven-jar-plugin</artifactId>
+        <groupId>org.apache.avro</groupId>
+        <artifactId>avro-maven-plugin</artifactId>
         <executions>
           <execution>
+            <phase>generate-test-sources</phase>
             <goals>
-              <goal>test-jar</goal>
+              <goal>schema</goal>
             </goals>
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-jar-plugin</artifactId>
+      </plugin>
     </plugins>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <groupId>org.apache.avro</groupId>
+          <artifactId>avro-maven-plugin</artifactId>
+          <version>${avro.version}</version>
+          <executions>
+            <execution>
+              <phase>generate-test-sources</phase>
+              <goals>
+                <goal>schema</goal>
+              </goals>
+            </execution>
+          </executions>
+          <configuration>
+            <testSourceDirectory>${project.basedir}/src/test/resources</testSourceDirectory>
+            <testOutputDirectory>${project.basedir}/src/test/gen</testOutputDirectory>
+            <enableDecimalLogicalType>true</enableDecimalLogicalType>
+            <stringType>String</stringType>
+          </configuration>
+        </plugin>
+      </plugins>
+    </pluginManagement>
   </build>
 </project>
diff --git a/kafka-handler/src/java/org/apache/hadoop/hive/kafka/KafkaSerDe.java b/kafka-handler/src/java/org/apache/hadoop/hive/kafka/KafkaSerDe.java
index ffe7788..892e56f 100644
--- a/kafka-handler/src/java/org/apache/hadoop/hive/kafka/KafkaSerDe.java
+++ b/kafka-handler/src/java/org/apache/hadoop/hive/kafka/KafkaSerDe.java
@@ -25,6 +25,7 @@ import org.apache.avro.generic.GenericDatumWriter;
 import org.apache.avro.generic.GenericRecord;
 import org.apache.avro.io.BinaryEncoder;
 import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
 import org.apache.avro.io.DecoderFactory;
 import org.apache.avro.io.EncoderFactory;
 import org.apache.avro.specific.SpecificDatumReader;
@@ -133,12 +134,46 @@ import java.util.stream.Collectors;
       Preconditions.checkArgument(!schemaFromProperty.isEmpty(), "Avro Schema is empty Can not go further");
       Schema schema = AvroSerdeUtils.getSchemaFor(schemaFromProperty);
       LOG.debug("Building Avro Reader with schema {}", schemaFromProperty);
-      bytesConverter = new AvroBytesConverter(schema);
+      bytesConverter = getByteConverterForAvroDelegate(schema, tbl);
     } else {
       bytesConverter = new BytesWritableConverter();
     }
   }
 
+  enum BytesConverterType {
+    SKIP,
+    NONE;
+
+    static BytesConverterType fromString(String value) {
+      try {
+        return BytesConverterType.valueOf(value.trim().toUpperCase());
+      } catch (Exception e){
+        return NONE;
+      }
+    }
+  }
+
+  BytesConverter getByteConverterForAvroDelegate(Schema schema, Properties tbl) throws SerDeException {
+    String avroBytesConverterPropertyName = AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_TYPE.getPropName();
+    String avroBytesConverterProperty = tbl.getProperty(avroBytesConverterPropertyName,
+        BytesConverterType.NONE.toString());
+    BytesConverterType avroByteConverterType = BytesConverterType.fromString(avroBytesConverterProperty);
+    switch (avroByteConverterType) {
+    case SKIP:
+      String avroSkipBytesPropertyName = AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_SKIP_BYTES.getPropName();
+      Integer avroSkipBytes = 0;
+      try {
+        avroSkipBytes = Integer.parseInt(tbl.getProperty(avroSkipBytesPropertyName));
+      } catch (NumberFormatException e) {
+        String message = "Value of " + avroSkipBytesPropertyName + " could not be parsed into an integer properly.";
+        throw new SerDeException(message, e);
+      }
+      return new AvroSkipBytesConverter(schema, avroSkipBytes);
+    case NONE: return new AvroBytesConverter(schema);
+    default: throw new SerDeException("Value of " + avroBytesConverterPropertyName + " was invalid.");
+    }
+  }
+
   @Override public Class<? extends Writable> getSerializedClass() {
     return delegateSerDe.getSerializedClass();
   }
@@ -327,7 +362,7 @@ import java.util.stream.Collectors;
     K getWritable(byte[] value);
   }
 
-  private static class AvroBytesConverter implements BytesConverter<AvroGenericRecordWritable> {
+  static class AvroBytesConverter implements BytesConverter<AvroGenericRecordWritable> {
     private final Schema schema;
     private final DatumReader<GenericRecord> dataReader;
     private final GenericDatumWriter<GenericRecord> gdw = new GenericDatumWriter<>();
@@ -354,12 +389,18 @@ import java.util.stream.Collectors;
       return valueBytes;
     }
 
+    Decoder getDecoder(byte[] value) throws SerDeException {
+      return DecoderFactory.get().binaryDecoder(value, null);
+    }
+
     @Override public AvroGenericRecordWritable getWritable(byte[] value) {
       GenericRecord avroRecord = null;
       try {
-        avroRecord = dataReader.read(null, DecoderFactory.get().binaryDecoder(value, null));
+        avroRecord = dataReader.read(null, getDecoder(value));
       } catch (IOException e) {
         Throwables.propagate(new SerDeException(e));
+      } catch (SerDeException e) {
+        Throwables.propagate(e);
       }
 
       avroGenericRecordWritable.setRecord(avroRecord);
@@ -369,6 +410,30 @@ import java.util.stream.Collectors;
     }
   }
 
+    /**
+     * Avro converter which skips the first @skipBytes of each message.
+     *
+     * This may be needed for various serializers, such as the Confluent Avro serializer, which uses the first five
+     * bytes to indicate a magic byte, as well as a four byte schema ID.
+     */
+  static class AvroSkipBytesConverter extends AvroBytesConverter {
+    private final int skipBytes;
+
+    AvroSkipBytesConverter(Schema schema, int skipBytes) {
+      super(schema);
+      this.skipBytes = skipBytes;
+    }
+
+    @Override
+    Decoder getDecoder(byte[] value) throws SerDeException {
+      try {
+        return DecoderFactory.get().binaryDecoder(value, this.skipBytes, value.length - this.skipBytes, null);
+      } catch (ArrayIndexOutOfBoundsException e) {
+        throw new SerDeException("Skip bytes value is larger than the message length.", e);
+      }
+    }
+  }
+
   private static class BytesWritableConverter implements BytesConverter<BytesWritable> {
     @Override public byte[] getBytes(BytesWritable writable) {
       return writable.getBytes();
diff --git a/kafka-handler/src/test/org/apache/hadoop/hive/kafka/AvroBytesConverterTest.java b/kafka-handler/src/test/org/apache/hadoop/hive/kafka/AvroBytesConverterTest.java
new file mode 100644
index 0000000..e0c4b28
--- /dev/null
+++ b/kafka-handler/src/test/org/apache/hadoop/hive/kafka/AvroBytesConverterTest.java
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.kafka;
+
+import com.google.common.collect.Maps;
+
+import io.confluent.kafka.schemaregistry.client.MockSchemaRegistryClient;
+import io.confluent.kafka.serializers.AbstractKafkaAvroSerDeConfig;
+import io.confluent.kafka.serializers.KafkaAvroSerializer;
+
+import org.apache.avro.Schema;
+
+import org.apache.hadoop.hive.serde2.avro.AvroGenericRecordWritable;
+import org.apache.hadoop.hive.serde2.avro.AvroSerdeUtils;
+import org.apache.hadoop.hive.serde2.SerDeException;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+
+/**
+ * Test class for Hive Kafka Avro SerDe with variable bytes skipped.
+ */
+public class AvroBytesConverterTest {
+  private static SimpleRecord simpleRecord = SimpleRecord.newBuilder().setId("123").setName("test").build();
+  private static byte[] simpleRecordConfluentBytes;
+
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
+  /**
+   * Use the KafkaAvroSerializer from Confluent to serialize the simpleRecord.
+   */
+  @BeforeClass
+  public static void setUp() {
+    Map<String, String> config = Maps.newHashMap();
+    config.put(AbstractKafkaAvroSerDeConfig.SCHEMA_REGISTRY_URL_CONFIG, "http://localhost:8081");
+    KafkaAvroSerializer avroSerializer = new KafkaAvroSerializer(new MockSchemaRegistryClient());
+    avroSerializer.configure(config, false);
+    simpleRecordConfluentBytes = avroSerializer.serialize("temp", simpleRecord);
+  }
+
+  private void runConversionTest(Properties tbl, byte[] serializedSimpleRecord) throws SerDeException {
+    KafkaSerDe serde = new KafkaSerDe();
+    Schema schema = SimpleRecord.getClassSchema();
+    KafkaSerDe.AvroBytesConverter conv = (KafkaSerDe.AvroBytesConverter)serde.getByteConverterForAvroDelegate(
+        schema, tbl);
+    AvroGenericRecordWritable simpleRecordWritable = conv.getWritable(serializedSimpleRecord);
+
+    Assert.assertNotNull(simpleRecordWritable);
+    Assert.assertEquals(SimpleRecord.class, simpleRecordWritable.getRecord().getClass());
+
+    SimpleRecord simpleRecordDeserialized = (SimpleRecord) simpleRecordWritable.getRecord();
+
+    Assert.assertNotNull(simpleRecordDeserialized);
+    Assert.assertEquals(simpleRecord, simpleRecordDeserialized);
+  }
+
+  /**
+   * Tests the default case of no skipped bytes per record works properly.
+   */
+  @Test
+  public void convertWithAvroBytesConverter() throws SerDeException {
+    // Since the serialized version was created by Confluent,
+    // let's remove the first five bytes to get the actual message.
+    int recordLength = simpleRecordConfluentBytes.length;
+    byte[] simpleRecordWithNoOffset = Arrays.copyOfRange(simpleRecordConfluentBytes, 5, recordLength);
+
+    Properties tbl = new Properties();
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_TYPE.getPropName(), "NONE");
+
+    runConversionTest(tbl, simpleRecordWithNoOffset);
+  }
+
+  /**
+   * Tests that the skip converter skips 5 bytes properly, which matches what Confluent needs.
+   */
+  @Test
+  public void convertWithConfluentAvroBytesConverter() throws SerDeException {
+    Integer offset = 5;
+
+    Properties tbl = new Properties();
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_TYPE.getPropName(), "SKIP");
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_SKIP_BYTES.getPropName(), offset.toString());
+
+    runConversionTest(tbl, simpleRecordConfluentBytes);
+  }
+
+  /**
+   * Tests that the skip converter skips a custom number of bytes properly.
+   */
+  @Test
+  public void convertWithCustomAvroSkipBytesConverter() throws SerDeException {
+    Integer offset = 2;
+    // Remove all but two bytes of the five byte offset which Confluent adds,
+    // to simulate a message with only 2 bytes in front of each message.
+    int recordLength = simpleRecordConfluentBytes.length;
+    byte[] simpleRecordAsOffsetBytes = Arrays.copyOfRange(simpleRecordConfluentBytes, 5 - offset, recordLength);
+
+    Properties tbl = new Properties();
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_TYPE.getPropName(), "SKIP");
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_SKIP_BYTES.getPropName(), offset.toString());
+
+    runConversionTest(tbl, simpleRecordAsOffsetBytes);
+  }
+
+  /**
+   * Test that when we skip more bytes than are in the message, we throw an exception properly.
+   */
+  @Test
+  public void skipBytesLargerThanMessageSizeConverter() throws SerDeException {
+    // The simple record we are serializing is two strings, that combine to be 7 characters or 14 bytes.
+    // Adding in the 5 byte offset, we get 19 bytes. To make sure we go bigger than that, we are setting
+    // the offset to ten times that value.
+    Integer offset = 190;
+
+    Properties tbl = new Properties();
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_TYPE.getPropName(), "SKIP");
+    tbl.setProperty(AvroSerdeUtils.AvroTableProperties.AVRO_SERDE_SKIP_BYTES.getPropName(), offset.toString());
+
+    exception.expect(RuntimeException.class);
+    exception.expectMessage("org.apache.hadoop.hive.serde2.SerDeException: " +
+        "Skip bytes value is larger than the message length.");
+    runConversionTest(tbl, simpleRecordConfluentBytes);
+  }
+
+  /**
+  * Test that we properly parse the converter type, no matter the casing.
+  */
+  @Test
+  public void bytesConverterTypeParseTest() {
+    Map<String, KafkaSerDe.BytesConverterType> testCases = new HashMap<String, KafkaSerDe.BytesConverterType>() {{
+        put("skip", KafkaSerDe.BytesConverterType.SKIP);
+        put("sKIp", KafkaSerDe.BytesConverterType.SKIP);
+        put("SKIP", KafkaSerDe.BytesConverterType.SKIP);
+        put("   skip   ", KafkaSerDe.BytesConverterType.SKIP);
+        put("SKIP1", KafkaSerDe.BytesConverterType.NONE);
+        put("skipper", KafkaSerDe.BytesConverterType.NONE);
+        put("", KafkaSerDe.BytesConverterType.NONE);
+        put(null, KafkaSerDe.BytesConverterType.NONE);
+        put("none", KafkaSerDe.BytesConverterType.NONE);
+        put("NONE", KafkaSerDe.BytesConverterType.NONE);
+      }};
+
+    for(Map.Entry<String, KafkaSerDe.BytesConverterType> entry: testCases.entrySet()) {
+      Assert.assertEquals(entry.getValue(), KafkaSerDe.BytesConverterType.fromString(entry.getKey()));
+    }
+  }
+}
diff --git a/kafka-handler/src/test/resources/SimpleRecord.avsc b/kafka-handler/src/test/resources/SimpleRecord.avsc
new file mode 100644
index 0000000..47b6156
--- /dev/null
+++ b/kafka-handler/src/test/resources/SimpleRecord.avsc
@@ -0,0 +1,13 @@
+{
+  "type" : "record",
+  "name" : "SimpleRecord",
+  "namespace" : "org.apache.hadoop.hive.kafka",
+  "fields" : [ {
+    "name" : "id",
+    "type" : "string"
+  }, {
+    "name" : "name",
+    "type" : "string"
+  }
+  ]
+}
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java b/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java
index d16abdb..b540073 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java
@@ -68,6 +68,8 @@ public class AvroSerdeUtils {
     SCHEMA_NAME("avro.schema.name"),
     SCHEMA_DOC("avro.schema.doc"),
     AVRO_SERDE_SCHEMA("avro.serde.schema"),
+    AVRO_SERDE_TYPE("avro.serde.type"),
+    AVRO_SERDE_SKIP_BYTES("avro.serde.skip.bytes"),
     SCHEMA_RETRIEVER("avro.schema.retriever");
 
     private final String propName;