[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-26 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14075475#comment-14075475
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/26/14 7:46 PM:
---

Agreed, makes sense - attached patch.
I wonder why I did not receive an email on gmail with your comment, since I am 
subscribed (watching) this issue, do you have any idea why?


was (Author: nmarasoi):
attached

 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: nicu marasoiu
 Attachments: bringAllBrokers.patch


 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-21 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14068442#comment-14068442
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/21/14 12:07 PM:


2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.

Agree with both comments, and anticipated this second one :)


was (Author: nmarasoi):
2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.

 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: Jun Rao
 Attachments: 
 KAFKA-1549__dead_brokers_coming_in_the_TopicMetadataResponse_.patch


 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-21 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14068442#comment-14068442
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/21/14 12:09 PM:


2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.

Agree with both comments, and anticipated this second one :) - is this locking 
encapsulation useful to make it safe and easy to reason on the multithreading 
behavior, or not only that?


was (Author: nmarasoi):
2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.

Agree with both comments, and anticipated this second one :)

 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: Jun Rao

 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-21 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14068442#comment-14068442
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/21/14 5:53 PM:
---

Implemented point 1 and reverted for point 2, attached patch.

2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.
I understand that this locking encapsulation useful to make it safe and easy to 
reason on the multithreading behavior, that is the reason right?



was (Author: nmarasoi):
2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.

Agree with both comments, and anticipated this second one :) - is this locking 
encapsulation useful to make it safe and easy to reason on the multithreading 
behavior, or not only that?

 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: Jun Rao
 Attachments: 
 kafka-1549__only_last_seen_alive_brokers_to_be_responded_part_of_the_topic_metadata_refres.patch


 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-21 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14068442#comment-14068442
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/21/14 6:05 PM:
---

Implemented point 1 and reverted for point 2, attached patch.
I used implicit parameter for the rwLock, so it required to make the lock the 
second argument-list to compile. I think in most cases there will be a single 
implicit read write lock reference available.

2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.
I understand that this locking encapsulation useful to make it safe and easy to 
reason on the multithreading behavior, that is the reason right?



was (Author: nmarasoi):
Implemented point 1 and reverted for point 2, attached patch.

2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.
I understand that this locking encapsulation useful to make it safe and easy to 
reason on the multithreading behavior, that is the reason right?


 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: Jun Rao
 Attachments: 
 kafka-1549__only_last_seen_alive_brokers_to_be_responded_part_of_the_topic_metadata_refres.patch


 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Comment Edited] (KAFKA-1549) dead brokers coming in the TopicMetadataResponse

2014-07-21 Thread nicu marasoiu (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-1549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14068442#comment-14068442
 ] 

nicu marasoiu edited comment on KAFKA-1549 at 7/21/14 6:06 PM:
---

Implemented point 1 and reverted for point 2, attached patch.
I used implicit parameter for the rwLock, so it required to make the lock the 
second argument-list to compile. I think in most cases there will be a single 
implicit read write lock reference available.

def inReadLock[T](fun: = T)(lock: ReadWriteLock): T = {..

2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.
I understand that this locking encapsulation useful to make it safe and easy to 
reason on the multithreading behavior, that is the reason right?



was (Author: nmarasoi):
Implemented point 1 and reverted for point 2, attached patch.
I used implicit parameter for the rwLock, so it required to make the lock the 
second argument-list to compile. I think in most cases there will be a single 
implicit read write lock reference available.

2. Indeed, to avoid that overhead and make it atomical (i.e. brokers and topic 
metadata from the same snapshot of metadata). However the atomicity is not 
required by the functionality.
I understand that this locking encapsulation useful to make it safe and easy to 
reason on the multithreading behavior, that is the reason right?


 dead brokers coming in the TopicMetadataResponse
 

 Key: KAFKA-1549
 URL: https://issues.apache.org/jira/browse/KAFKA-1549
 Project: Kafka
  Issue Type: Bug
Affects Versions: 0.8.2
 Environment: trunk
Reporter: nicu marasoiu
Assignee: Jun Rao
 Attachments: 
 kafka-1549__only_last_seen_alive_brokers_to_be_responded_part_of_the_topic_metadata_refres.patch


 JunRao confirming my observation that brokers are only added to the 
 metadataCache, never removed: The way that we update liveBrokers in 
 MetadataCache.updateCache() doesn't seem right. We only add newly received 
 live brokers to the list. However, there could be existing brokers in that 
 list that are now dead. Those dead brokers shouldn't be returned to the 
 clients. We should probably just take the new live broker list and cache it.



--
This message was sent by Atlassian JIRA
(v6.2#6252)