Skip to content

[schema] introduce AUTO_PRODUCE schema - #2685

Merged
sijie merged 4 commits into
apache:masterfrom
sijie:fix_function_schema
Oct 3, 2018
Merged

[schema] introduce AUTO_PRODUCE schema#2685
sijie merged 4 commits into
apache:masterfrom
sijie:fix_function_schema

Conversation

@sijie

@sijie sijie commented Sep 30, 2018

Copy link
Copy Markdown
Member

Motivation

Currently trigger function is broken due to the schema enforcement we added recently:
A producer without schema can't produce messages into a topic with schema.

Changes

  • Rename AUTO to AUTO_CONSUME
  • Introduce AUTO_PRODUCE schema. The schema produces byte[], but it will validate
    the bytes are compatible with the schema associated with the topic before producing.
  • Change trigger function to use AUTO_PRODUCE schema.

*Motivation*

Java instance main is expecting health check interval in seconds.

*Changes

Make sure the health check interval is passed in seconds.

*Tests*

It fixes following issue

```
14:36:29.528 [FunctionActionerThread] INFO  org.apache.pulsar.functions.runtime.ProcessRuntime - Started process successfully
Exception in thread "main" java.lang.IllegalArgumentException
	at java.util.concurrent.ScheduledThreadPoolExecutor.scheduleAtFixedRate(ScheduledThreadPoolExecutor.java:565)
	at java.util.concurrent.Executors$DelegatedScheduledExecutorService.scheduleAtFixedRate(Executors.java:735)
	at org.apache.pulsar.functions.runtime.JavaInstanceMain.start(JavaInstanceMain.java:154)
	at org.apache.pulsar.functions.runtime.JavaInstanceMain.main(JavaInstanceMain.java:185)
```
*Motivation*

Currently trigger function is broken due to the schema enforcement we added recently:
A producer without schema can't produce messages into a topic with schema.

*Changes*

- Rename `AUTO` to `AUTO_CONSUME`
- Introduce `AUTO_PRODUCE` schema. The schema produces `byte[]`, but it will validate
  the bytes are compatible with the schema associated with the topic before producing.
- Change trigger function to use `AUTO_PRODUCE` schema.
@sijie sijie added type/bug The PR fixed a bug or issue reported a bug type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/function component/schemaregistry labels Sep 30, 2018
@sijie sijie added this to the 2.3.0 milestone Sep 30, 2018
@sijie sijie self-assigned this Sep 30, 2018
@sijie

sijie commented Sep 30, 2018

Copy link
Copy Markdown
Member Author

this pr is based on #2684


Consumer<GenericRecord> consumer = pulsarClient
.newConsumer(Schema.AUTO())
.newConsumer(Schema.AUTO_CONSUME())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are these methods, and not fields? It looks weird.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made them methods to be consistent with AVRO and JSON.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AVRO and JSON take arguments though. A static method that takes no arguments is effectively a constant, and if it's not, it's doing something very suspicious in the background.

Anyhow, I wouldn't hold up the change for it, but I think it would be better as a constant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually it can't be a constant. because it needs to set SchemaInfo when the client fetches schema info from brokers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same applied to both AUTO_CONSUME() and AUTO_PRODUCE

ensureSchemaInitialized();

// verify if the message can be decoded by the underlying schema
return schema.encode(schema.decode(message));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be a bit more efficient only decode to check, and if it can decode, return the original byte[]

schema.decode(message);
return message;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

@sijie

sijie commented Oct 3, 2018

Copy link
Copy Markdown
Member Author

run java8 tests

@sijie

sijie commented Oct 3, 2018

Copy link
Copy Markdown
Member Author

run java8 tests

@sijie
sijie merged commit 1f37326 into apache:master Oct 3, 2018
@sijie
sijie deleted the fix_function_schema branch October 3, 2018 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/function type/bug The PR fixed a bug or issue reported a bug type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants