Skip to content
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

SAMZA-1821: fix the IllegalStateException complaining duplicate key when fetching systemStream configs #616

Closed
wants to merge 2 commits into from

Conversation

weiqingy
Copy link
Contributor

@weiqingy weiqingy commented Aug 24, 2018

What changes were proposed in this pull request?

This PR is to fix IllegalStateException complaining duplicate key when fetching resource configs in SamzaSqlApplicationConfig.

How was this patch tested?

Pass the local build and current unit tests.
Added a new unit test.

@@ -113,13 +113,13 @@ public SamzaSqlApplicationConfig(Config staticConfig) {
inputSystemStreamConfigBySource = queryInfo.stream()
.map(QueryInfo::getSources)
.flatMap(Collection::stream)
.collect(Collectors.toMap(Function.identity(), src -> ioResolver.fetchSourceInfo(src)));
.collect(Collectors.toMap(Function.identity(), ioResolver::fetchSourceInfo, (src1, src2) -> src1));
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we use distinct() before collect() to save some resolver calls to fetch source information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right. thanks for pointing this out.


Set<SqlIOConfig> systemStreamConfigs = new HashSet<>(inputSystemStreamConfigBySource.values());

outputSystemStreamConfigsBySource = queryInfo.stream()
.map(QueryInfo::getSink)
.collect(Collectors.toMap(Function.identity(), x -> ioResolver.fetchSinkInfo(x)));
.collect(Collectors.toMap(Function.identity(), ioResolver::fetchSinkInfo, (src1, src2) -> src1));
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above.

@@ -24,7 +24,6 @@
import java.io.PrintWriter;
import java.util.List;

import org.apache.samza.sql.testutil.SqlFileParser;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just cleaning up code when I see it.

Copy link
Contributor

@srinipunuru srinipunuru left a comment

Choose a reason for hiding this comment

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

LGTM

@asfgit asfgit closed this in b0b2922 Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants