[improve][broker]Part-3 of PIP-433: always allow replicator to register a new compatible schema#25461
[improve][broker]Part-3 of PIP-433: always allow replicator to register a new compatible schema#25461poorbarcode wants to merge 6 commits intoapache:masterfrom
Conversation
| boolean isReplicatorProducer = false; | ||
| if (commandGetOrCreateSchema.hasProducerName()) { | ||
| isReplicatorProducer = Producer.isRemoteOrShadow(commandGetOrCreateSchema.getProducerName(), | ||
| getBrokerService().getPulsar().getConfig().getReplicatorPrefix()); | ||
| } | ||
| CompletableFuture<SchemaVersion> schemaVersionFuture = | ||
| tryAddSchema(topic, schema, isReplicatorProducer); |
There was a problem hiding this comment.
Will it be a security issue that user can just set producer name to pulsar.repl.* to get rid of the schema upload control? if user disabled is_allow_auto_update_schema
| @Option(names = { "--disable-for-replicator" }, | ||
| description = "By default, brokers always allow replicator to register new compatible schemas even" | ||
| + " when auto updates are disabled, if you want to disable replicators to register compatible schemas," | ||
| + " please set it to true") | ||
| private boolean disableForReplicator; | ||
|
|
There was a problem hiding this comment.
--disable-for-replicator is a primitive boolean defaulting to false, so every set-is-allow-auto-update-schema call sends allowAutoUpdateSchemaWithReplicator=true unless the flag is supplied. A user who ran ... --disable --disable-for-replicator and later runs ... --enable silently re-enables replicator updates — different from the REST API's null-preserving semantics.
There was a problem hiding this comment.
Please also check the Java API
| && schema_compatibility_strategy == other.schema_compatibility_strategy | ||
| && is_allow_auto_update_schema == other.is_allow_auto_update_schema | ||
| && is_allow_auto_update_schema_with_replicator | ||
| == other.is_allow_auto_update_schema_with_replicator |
There was a problem hiding this comment.
Should use Objects.equals()?
There was a problem hiding this comment.
Good suggestion
| description = "By default, brokers always allow replicator to register new compatible schemas even" | ||
| + " when auto updates are disabled, if you want to disable replicators to register compatible schemas," | ||
| + " please set it to true") | ||
| private boolean disableForReplicator; |
There was a problem hiding this comment.
PIP says CLI flag is --enable-for-replicator, implementation uses --disable-for-replicator.
| } | ||
| return policies; | ||
| }, (policies) -> policies.is_allow_auto_update_schema, | ||
| "isAllowAutoUpdateSchema"); |
There was a problem hiding this comment.
Missed isAllowAutoUpdateSchemaWithReplicator?
There was a problem hiding this comment.
It has been modified at the line-2583
…er a new compatible schema
|
@poorbarcode @codelipenghui When geo-replicator creates a remote producer in the broker, it indeed carries the replicator's producer name and uses Schema.AUTO_PRODUCE_BYTES(). But the key point of AUTO_PRODUCE_BYTES is: when establishing a producer connection, it doesn't send the "specific schema of this message" all at once. The actual specific schema registration happens later during the message sending process, when encountering a new schema, it then triggers GET_OR_CREATE_SCHEMA. This call chain looks like this locally:
|
3 participants
Motivation
This is part-3 of PIP-433, see the goal 1 in PIP-433
Modifications
always allow the replicator to register a new compatible schema, users can turn off this function
Documentation
docdoc-requireddoc-not-neededdoc-complete