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

add partitions to multiple topics at once #98

Merged
merged 5 commits into from
Jul 30, 2015

Conversation

jisookim0513
Copy link
Contributor

No description provided.

readVersions: Map[String,Int]) {
// topicAndReplicaList is sorted by number of partitions each topic has in order not to start adding partitions if any of requests doesn't work with newNumPartitions
for ((topic, replicaList) <- topicAndReplicaList) {
addPartitions(curator, topic, newNumPartitions, replicaList, brokerList, readVersions.getOrElse(topic,-1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably should skip topics for which we don't have a readVersion to avoid potential issues with two concurrent add partitions to topics commands. You can return a list of topics for which no read version was found or some form of error object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll return an error in this case with a list of topics with no read version.

@patelh
Copy link
Collaborator

patelh commented Jul 19, 2015

Can we add test for this?

@jisookim0513
Copy link
Contributor Author

Added a test for addPartitionsToMultipleTopics under TestKafkaManager.

checkCondition(topicsWithoutReadVersion.isEmpty, TopicErrors.NoReadVersionFound(topicsWithoutReadVersion.mkString(", ")))

// topicAndReplicaList is sorted by number of partitions each topic has in order not to start adding partitions if any of requests doesn't work with newNumPartitions
for ((topic, replicaList) <- topicAndReplicaList) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are already doing the readVersion check, you can do something like this here:

for {
(topic, replicaList) <- topicAndReplicaList
readVersion = readVersions(topic)
} {
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

patelh added a commit that referenced this pull request Jul 30, 2015
add partitions to multiple topics at once
@patelh patelh merged commit 4d762db into yahoo:master Jul 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants