close
Skip to content

[hotfix][redis] fix npe cause by null host parameter#8881

Merged
liunaijie merged 5 commits into
apache:devfrom
fcb-xiaobo:feature/optimized_redis_connection_param
Mar 12, 2025
Merged

[hotfix][redis] fix npe cause by null host parameter#8881
liunaijie merged 5 commits into
apache:devfrom
fcb-xiaobo:feature/optimized_redis_connection_param

Conversation

@fcb-xiaobo

Copy link
Copy Markdown
Collaborator

Purpose of this pull request

image
When the host parameter is not filled in, the config. get method will throw a null pointer exception

Does this PR introduce any user-facing change?

fix #8841

How was this patch tested?

Check list

@Hisoka-X

Hisoka-X commented Mar 3, 2025

Copy link
Copy Markdown
Member

When the host parameter is not filled in, the config. get method will throw a null pointer exception

Could you provide the error stack?

@fcb-xiaobo

fcb-xiaobo commented Mar 3, 2025

Copy link
Copy Markdown
Collaborator Author

When the host parameter is not filled in, the config. get method will throw a null pointer exception

Could you provide the error stack?

image

image

stack info
Exception in thread "main" org.apache.seatunnel.core.starter.exception.CommandExecuteException: SeaTunnel job executed failed
at org.apache.seatunnel.core.starter.seatunnel.command.ClientExecuteCommand.execute(ClientExecuteCommand.java:228)
at org.apache.seatunnel.core.starter.SeaTunnel.run(SeaTunnel.java:40)
at org.apache.seatunnel.example.engine.SeaTunnelEngineLocalExample.main(SeaTunnelEngineLocalExample.java:52)
Caused by: org.apache.seatunnel.api.table.factory.FactoryException: ErrorCode:[API-06], ErrorDescription:[Factory initialize failed] - Unable to create a source for identifier 'Redis'.
at org.apache.seatunnel.api.table.factory.FactoryUtil.restoreAndPrepareSource(FactoryUtil.java:168)
at org.apache.seatunnel.api.table.factory.FactoryUtil.createAndPrepareSource(FactoryUtil.java:86)
at org.apache.seatunnel.engine.core.parse.MultipleTableJobConfigParser.parseSource(MultipleTableJobConfigParser.java:376)
at org.apache.seatunnel.engine.core.parse.MultipleTableJobConfigParser.parse(MultipleTableJobConfigParser.java:227)
at org.apache.seatunnel.engine.client.job.ClientJobExecutionEnvironment.getLogicalDag(ClientJobExecutionEnvironment.java:123)
at org.apache.seatunnel.engine.client.job.ClientJobExecutionEnvironment.execute(ClientJobExecutionEnvironment.java:191)
at org.apache.seatunnel.core.starter.seatunnel.command.ClientExecuteCommand.execute(ClientExecuteCommand.java:165)
... 2 more
Caused by: java.lang.NullPointerException
at org.apache.seatunnel.connectors.seatunnel.redis.config.RedisParameters.buildWithConfig(RedisParameters.java:72)
at org.apache.seatunnel.connectors.seatunnel.redis.source.RedisSource.(RedisSource.java:56)
at org.apache.seatunnel.connectors.seatunnel.redis.source.RedisSourceFactory.lambda$createSource$0(RedisSourceFactory.java:45)
at org.apache.seatunnel.api.table.factory.FactoryUtil.createAndPrepareSource(FactoryUtil.java:180)
at org.apache.seatunnel.api.table.factory.FactoryUtil.restoreAndPrepareSource(FactoryUtil.java:136)
... 8 more
[] 2025-03-03 18:10:02,703 DEBUG org.apache.hadoop.util.ShutdownHookManager - ShutdownHookManger completed shutdown.
Disconnected from the target VM, address: '127.0.0.1:55053', transport: 'socket'

Process finished with exit code 1

@Hisoka-X Hisoka-X left a comment

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.

@fcb-xiaobo

Copy link
Copy Markdown
Collaborator Author

Maybe change port from int to Integer is a better option. https://github.com/apache/seatunnel/pull/8881/files#diff-85c72df044b54265682571cdb07752eb396fdf68965cbcf863553efa9a22b34fR49

Okay, thank you for your suggestion. After rounding it to Integer, I can run it locally

public class RedisParameters implements Serializable {
private String host;
private int port;
private Integer port;

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.

Let's add some test case for RedisParameters.

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.

How about update it to redis default port 6379.

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.

How about update it to redis default port 6379.

In cluster mode it should be null.

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.

How about update it to redis default port 6379.

In cluster mode it should be null.

In cluster moe, it won't be use
https://github.com/apache/seatunnel/pull/8841/files#diff-85c72df044b54265682571cdb07752eb396fdf68965cbcf863553efa9a22b34fR175

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.

OK, let's update it to 6379 as default. @fcb-xiaobo

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Okay, thank you for your suggestions. I will update the code and documentation later

@liunaijie liunaijie merged commit 7bd5865 into apache:dev Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants