New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the multi version schema support #3876
Changes from 8 commits
54d58c0
2776e32
9714c9d
9447588
b01f509
5dae355
ee531dc
cea87d0
70ca1fe
c491e42
ed51af9
dd838ce
9597bc0
7571d7e
c58da0b
a2c45bf
d6b908a
d6c90bb
10af2c2
a3b6966
4295758
5abb561
d8a29a0
3350a80
18a9170
b74beed
c939809
db9cb75
0c7f5c4
3c5a03a
59a260c
468a861
ff0aebf
c2887f2
291ab9f
42407b9
5e4a991
c30f3f9
ffeec02
4ff3214
8dd3d79
c2e7724
2f87051
e210c86
564dcee
66819a7
cd5b485
0691bf3
091f69c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
package org.apache.pulsar.client.impl.schema.generic; | ||
package org.apache.pulsar.client.api.schema; | ||
|
||
import org.apache.pulsar.client.api.Schema; | ||
|
||
|
@@ -31,6 +31,12 @@ public interface SchemaProvider<T> { | |
* @param schemaVersion schema version | ||
* @return schema instance of the provided <tt>schemaVersion</tt> | ||
*/ | ||
Schema<T> getSchema(byte[] schemaVersion); | ||
Schema<T> getVersionSchema(byte[] schemaVersion); | ||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add a blank line before |
||
* Retrieve the current schema. | ||
* | ||
* @return the current schema | ||
*/ | ||
Schema<T> getCurrentSchema() throws InterruptedException; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,8 @@ | |
import org.apache.pulsar.client.api.SubscriptionInitialPosition; | ||
import org.apache.pulsar.client.api.SubscriptionType; | ||
import org.apache.pulsar.client.impl.conf.ConsumerConfigurationData; | ||
import org.apache.pulsar.client.impl.schema.AvroSchema; | ||
import org.apache.pulsar.client.impl.schema.generic.MultiVersionGenericSchemaProvider; | ||
import org.apache.pulsar.common.api.Commands; | ||
import org.apache.pulsar.common.api.EncryptionContext; | ||
import org.apache.pulsar.common.api.EncryptionContext.EncryptionKey; | ||
|
@@ -152,6 +154,16 @@ enum SubscriptionMode { | |
static <T> ConsumerImpl<T> newConsumerImpl(PulsarClientImpl client, String topic, ConsumerConfigurationData<T> conf, | ||
ExecutorService listenerExecutor, int partitionIndex, CompletableFuture<Consumer<T>> subscribeFuture, | ||
SubscriptionMode subscriptionMode, MessageId startMessageId, Schema<T> schema, ConsumerInterceptors<T> interceptors) { | ||
if (schema != null && schema.supportSchemaVersioning()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually I am bit confused about the logic here. why do we need to cache this? Event we want to cache this, we should cache the |
||
Map<String, Schema> supportSchemaVersioningSchemaCache = client.getSupportSchemaVersioningSchemaCache(); | ||
Schema<T> schemaFromCache = supportSchemaVersioningSchemaCache.get(topic); | ||
if (schemaFromCache == null) { | ||
((AvroSchema<T>) schema).setSchemaProvider(new MultiVersionGenericSchemaProvider(TopicName.get(topic), client)); | ||
supportSchemaVersioningSchemaCache.put(topic, schema); | ||
} else { | ||
schema = schemaFromCache; | ||
} | ||
} | ||
if (conf.getReceiverQueueSize() == 0) { | ||
return new ZeroQueueConsumerImpl<>(client, topic, conf, listenerExecutor, partitionIndex, subscribeFuture, | ||
subscriptionMode, startMessageId, schema, interceptors); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,6 +83,7 @@ public class PulsarClientImpl implements PulsarClient { | |
private final ConnectionPool cnxPool; | ||
private final Timer timer; | ||
private final ExecutorProvider externalExecutorProvider; | ||
private Map<String, Schema> supportSchemaVersioningSchemaCache = new HashMap<>(); | ||
congbobo184 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. either use a concurrent structure or you should synchronize on accessing |
||
|
||
enum State { | ||
Open, Closing, Closed | ||
|
@@ -736,4 +737,8 @@ private static Mode convertRegexSubscriptionMode(RegexSubscriptionMode regexSubs | |
return null; | ||
} | ||
} | ||
|
||
public Map<String, Schema> getSupportSchemaVersioningSchemaCache(){ | ||
return this.supportSchemaVersioningSchemaCache; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,9 @@ | |
*/ | ||
package org.apache.pulsar.client.impl.schema; | ||
|
||
import com.google.common.cache.CacheBuilder; | ||
import com.google.common.cache.CacheLoader; | ||
import com.google.common.cache.LoadingCache; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.apache.avro.Conversions; | ||
import org.apache.avro.data.TimeConversions; | ||
|
@@ -30,26 +33,42 @@ | |
import org.apache.avro.reflect.ReflectDatumWriter; | ||
import org.apache.pulsar.client.api.SchemaSerializationException; | ||
import org.apache.pulsar.client.api.schema.SchemaDefinition; | ||
import org.apache.pulsar.client.impl.schema.generic.GenericAvroSchema; | ||
import org.apache.pulsar.client.impl.schema.generic.MultiVersionGenericSchemaProvider; | ||
import org.apache.pulsar.common.schema.SchemaInfo; | ||
import org.apache.pulsar.common.schema.SchemaType; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.io.ByteArrayOutputStream; | ||
import java.io.IOException; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.Map; | ||
import java.util.concurrent.ExecutionException; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
/** | ||
* An AVRO schema implementation. | ||
*/ | ||
@Slf4j | ||
public class AvroSchema<T> extends StructSchema<T> { | ||
private static final Logger LOG = LoggerFactory.getLogger(AvroSchema.class); | ||
|
||
private ReflectDatumWriter<T> datumWriter; | ||
private ReflectDatumReader<T> reader; | ||
private ReflectDatumReader<T> datumReader; | ||
private BinaryEncoder encoder; | ||
private ByteArrayOutputStream byteArrayOutputStream; | ||
|
||
private static final ThreadLocal<BinaryDecoder> decoders = | ||
new ThreadLocal<>(); | ||
private boolean supportSchemaVersioning; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think the code change here should be pushed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All right, it should push to StructSchema |
||
private final LoadingCache<byte[], ReflectDatumReader<T>> cache = CacheBuilder.newBuilder().maximumSize(100000) | ||
.expireAfterAccess(30, TimeUnit.MINUTES).build(new CacheLoader<byte[], ReflectDatumReader<T>>() { | ||
@Override | ||
public ReflectDatumReader<T> load(byte[] schemaVersion) throws Exception { | ||
return loadReader(schemaVersion); | ||
} | ||
}); | ||
|
||
// the aim to fix avro's bug | ||
// https://issues.apache.org/jira/browse/AVRO-1891 bug address explain | ||
// fix the avro logical type read and write | ||
|
@@ -85,8 +104,9 @@ private AvroSchema(org.apache.avro.Schema schema, | |
schemaDefinition.getProperties()); | ||
this.byteArrayOutputStream = new ByteArrayOutputStream(); | ||
this.encoder = EncoderFactory.get().binaryEncoder(this.byteArrayOutputStream, this.encoder); | ||
this.datumWriter = new ReflectDatumWriter<>(this.schema); | ||
this.reader = new ReflectDatumReader<>(this.schema); | ||
this.datumWriter = new ReflectDatumWriter<>(schema); | ||
this.datumReader = new ReflectDatumReader<>(schema); | ||
this.supportSchemaVersioning = schemaDefinition.getSupportSchemaVersioning(); | ||
} | ||
|
||
@Override | ||
|
@@ -110,9 +130,27 @@ public T decode(byte[] bytes) { | |
if (decoderFromCache == null) { | ||
decoders.set(decoder); | ||
} | ||
return reader.read(null, DecoderFactory.get().binaryDecoder(bytes, decoder)); | ||
return datumReader.read(null, DecoderFactory.get().binaryDecoder(bytes, decoder)); | ||
} catch (IOException e) { | ||
throw new SchemaSerializationException(e); | ||
} | ||
} | ||
|
||
@Override | ||
public T decode(byte[] bytes, byte[] schemaVersion) { | ||
try { | ||
BinaryDecoder decoderFromCache = decoders.get(); | ||
BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(bytes, decoderFromCache); | ||
if (decoderFromCache == null) { | ||
decoders.set(decoder); | ||
} | ||
return cache.get(schemaVersion).read(null, DecoderFactory.get().binaryDecoder(bytes, decoder)); | ||
} catch (IOException e) { | ||
throw new SchemaSerializationException(e); | ||
} catch (ExecutionException e) { | ||
LOG.error("Can't get generic schema for topic {} schema version {}", | ||
((MultiVersionGenericSchemaProvider)schemaProvider).getTopic().toString(), new String(schemaVersion, StandardCharsets.UTF_8), e); | ||
return null; | ||
} | ||
} | ||
|
||
|
@@ -135,4 +173,12 @@ public static <T> AvroSchema<T> of(Class<T> pojo, Map<String, String> properties | |
return new AvroSchema<>(createAvroSchema(schemaDefinition), schemaDefinition); | ||
} | ||
|
||
public boolean supportSchemaVersioning(){ | ||
return supportSchemaVersioning; | ||
} | ||
|
||
private ReflectDatumReader loadReader(byte[] schemaVersion) { | ||
return new ReflectDatumReader<T>(((GenericAvroSchema)schemaProvider.getVersionSchema(schemaVersion)).getAvroSchema(),schema); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getSchema
is probably okay. or we can call itgetSchemaByVersion