You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@samza.apache.org by Chris Riccomini <cr...@apache.org> on 2013/10/23 19:34:43 UTC

Review Request 14878: SAMZA-62

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/
-----------------------------------------------------------

Review request for samza.


Repository: samza


Description
-------

check error mapping for offset response in kafka checkpoint manager


Diffs
-----

  samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 

Diff: https://reviews.apache.org/r/14878/diff/


Testing
-------


Thanks,

Chris Riccomini

Re: Review Request 14878: SAMZA-62

Posted by Chris Riccomini <cr...@apache.org>.


> On Oct. 23, 2013, 5:54 p.m., Sriram Subramanian wrote:
> > You want to retry for all types of errors?

Good point. Looking at possible codes:

RETRY: val UnknownCode : Short = -1
OK: val NoError : Short = 0
NOT VALID: val OffsetOutOfRangeCode : Short = 1
FAIL: val InvalidMessageCode : Short = 2
RETRY: val UnknownTopicOrPartitionCode : Short = 3
NOT VALID: val InvalidFetchSizeCode  : Short = 4
RETRY: val LeaderNotAvailableCode : Short = 5
RETRY: val NotLeaderForPartitionCode : Short = 6
RETRY: val RequestTimedOutCode: Short = 7
RETRY: val BrokerNotAvailableCode: Short = 8
RETRY: val ReplicaNotAvailableCode: Short = 9
NOT VALID: val MessageSizeTooLargeCode: Short = 10
RETRY: val StaleControllerEpochCode: Short = 11

It seems like the only one where it makes sense to fully fail is when there's an invalid message code.

Debatable whether to retry on unknown code. Perhaps some are recoverable and some aren't?


- Chris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/#review27387
-----------------------------------------------------------


On Oct. 23, 2013, 5:34 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14878/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 5:34 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> check error mapping for offset response in kafka checkpoint manager
> 
> 
> Diffs
> -----
> 
>   samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 
> 
> Diff: https://reviews.apache.org/r/14878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>

Re: Review Request 14878: SAMZA-62

Posted by Sriram Subramanian <sr...@gmail.com>.


> On Oct. 23, 2013, 5:54 p.m., Sriram Subramanian wrote:
> > You want to retry for all types of errors?
> 
> Chris Riccomini wrote:
>     Good point. Looking at possible codes:
>     
>     RETRY: val UnknownCode : Short = -1
>     OK: val NoError : Short = 0
>     NOT VALID: val OffsetOutOfRangeCode : Short = 1
>     FAIL: val InvalidMessageCode : Short = 2
>     RETRY: val UnknownTopicOrPartitionCode : Short = 3
>     NOT VALID: val InvalidFetchSizeCode  : Short = 4
>     RETRY: val LeaderNotAvailableCode : Short = 5
>     RETRY: val NotLeaderForPartitionCode : Short = 6
>     RETRY: val RequestTimedOutCode: Short = 7
>     RETRY: val BrokerNotAvailableCode: Short = 8
>     RETRY: val ReplicaNotAvailableCode: Short = 9
>     NOT VALID: val MessageSizeTooLargeCode: Short = 10
>     RETRY: val StaleControllerEpochCode: Short = 11
>     
>     It seems like the only one where it makes sense to fully fail is when there's an invalid message code.
>     
>     Debatable whether to retry on unknown code. Perhaps some are recoverable and some aren't?

My list below. However, I think it might make the code ugly. If we have enough sleeps between retries we could simply retry for all error codes as long as we don't abuse the kafka servers. You could have a method shouldRetry and have this logic in there.

RETRY: val UnknownCode : Short = -1
OK: val NoError : Short = 0
FAIL: val OffsetOutOfRangeCode : Short = 1
FAIL: val InvalidMessageCode : Short = 2
FAIL: val UnknownTopicOrPartitionCode : Short = 3
FAIL: val InvalidFetchSizeCode  : Short = 4
RETRY: val LeaderNotAvailableCode : Short = 5
RETRY: val NotLeaderForPartitionCode : Short = 6
RETRY: val RequestTimedOutCode: Short = 7
RETRY: val BrokerNotAvailableCode: Short = 8
RETRY: val ReplicaNotAvailableCode: Short = 9
NOT VALID: val MessageSizeTooLargeCode: Short = 10
RETRY: val StaleControllerEpochCode: Short = 11


- Sriram


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/#review27387
-----------------------------------------------------------


On Oct. 23, 2013, 6:04 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14878/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:04 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> failing if we get an invalid message code
> 
> 
> check error mapping for offset response in kafka checkpoint manager
> 
> 
> Diffs
> -----
> 
>   samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 
> 
> Diff: https://reviews.apache.org/r/14878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>

Re: Review Request 14878: SAMZA-62

Posted by Sriram Subramanian <sr...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/#review27387
-----------------------------------------------------------


You want to retry for all types of errors?

- Sriram Subramanian


On Oct. 23, 2013, 5:34 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14878/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 5:34 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> check error mapping for offset response in kafka checkpoint manager
> 
> 
> Diffs
> -----
> 
>   samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 
> 
> Diff: https://reviews.apache.org/r/14878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>

Re: Review Request 14878: SAMZA-62

Posted by Sriram Subramanian <sr...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/#review27391
-----------------------------------------------------------

Ship it!


Ship It!

- Sriram Subramanian


On Oct. 23, 2013, 6:04 p.m., Chris Riccomini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14878/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:04 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> failing if we get an invalid message code
> 
> 
> check error mapping for offset response in kafka checkpoint manager
> 
> 
> Diffs
> -----
> 
>   samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 
> 
> Diff: https://reviews.apache.org/r/14878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Riccomini
> 
>

Re: Review Request 14878: SAMZA-62

Posted by Chris Riccomini <cr...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14878/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 6:04 p.m.)


Review request for samza.


Repository: samza


Description (updated)
-------

failing if we get an invalid message code


check error mapping for offset response in kafka checkpoint manager


Diffs (updated)
-----

  samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala a9ddc5ce5973f8c05630d7fd5702c99be6b62182 

Diff: https://reviews.apache.org/r/14878/diff/


Testing
-------


Thanks,

Chris Riccomini