Hey group, I am planning on starting a voting thread tomorrow. Please do reply if you feel there is anything left to discuss.
Best, Stanislav On Fri, Jul 27, 2018 at 11:05 PM Stanislav Kozlovski <stanis...@confluent.io> wrote: > Hey, Ray > > Thanks for pointing that out, it's fixed now > > Best, > Stanislav > > On Fri, Jul 27, 2018 at 9:43 PM Ray Chiang <rchi...@apache.org> wrote: > >> Thanks. Can you fix the link in the "KIPs under discussion" table on >> the main KIP landing page >> < >> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#>? >> >> I tried, but the Wiki won't let me. >> >> -Ray >> >> On 7/26/18 2:01 PM, Stanislav Kozlovski wrote: >> > Hey guys, >> > >> > @Colin - good point. I added some sentences mentioning recent >> improvements >> > in the introductory section. >> > >> > *Disk Failure* - I tend to agree with what Colin said - once a disk >> fails, >> > you don't want to work with it again. As such, I've changed my mind and >> > believe that we should mark the LogDir (assume its a disk) as offline on >> > the first `IOException` encountered. This is the LogCleaner's current >> > behavior. We shouldn't change that. >> > >> > *Respawning Threads* - I believe we should never re-spawn a thread. The >> > correct approach in my mind is to either have it stay dead or never let >> it >> > die in the first place. >> > >> > *Uncleanable-partition-names metric* - Colin is right, this metric is >> > unneeded. Users can monitor the `uncleanable-partitions-count` metric >> and >> > inspect logs. >> > >> > >> > Hey Ray, >> > >> >> 2) I'm 100% with James in agreement with setting up the LogCleaner to >> >> skip over problematic partitions instead of dying. >> > I think we can do this for every exception that isn't `IOException`. >> This >> > will future-proof us against bugs in the system and potential other >> errors. >> > Protecting yourself against unexpected failures is always a good thing >> in >> > my mind, but I also think that protecting yourself against bugs in the >> > software is sort of clunky. What does everybody think about this? >> > >> >> 4) The only improvement I can think of is that if such an >> >> error occurs, then have the option (configuration setting?) to create a >> >> <log_segment>.skip file (or something similar). >> > This is a good suggestion. Have others also seen corruption be generally >> > tied to the same segment? >> > >> > On Wed, Jul 25, 2018 at 11:55 AM Dhruvil Shah <dhru...@confluent.io> >> wrote: >> > >> >> For the cleaner thread specifically, I do not think respawning will >> help at >> >> all because we are more than likely to run into the same issue again >> which >> >> would end up crashing the cleaner. Retrying makes sense for transient >> >> errors or when you believe some part of the system could have healed >> >> itself, both of which I think are not true for the log cleaner. >> >> >> >> On Wed, Jul 25, 2018 at 11:08 AM Ron Dagostino <rndg...@gmail.com> >> wrote: >> >> >> >>> <<<respawning threads is likely to make things worse, by putting you >> in >> >> an >> >>> infinite loop which consumes resources and fires off continuous log >> >>> messages. >> >>> Hi Colin. In case it could be relevant, one way to mitigate this >> effect >> >> is >> >>> to implement a backoff mechanism (if a second respawn is to occur then >> >> wait >> >>> for 1 minute before doing it; then if a third respawn is to occur wait >> >> for >> >>> 2 minutes before doing it; then 4 minutes, 8 minutes, etc. up to some >> max >> >>> wait time). >> >>> >> >>> I have no opinion on whether respawn is appropriate or not in this >> >> context, >> >>> but a mitigation like the increasing backoff described above may be >> >>> relevant in weighing the pros and cons. >> >>> >> >>> Ron >> >>> >> >>> On Wed, Jul 25, 2018 at 1:26 PM Colin McCabe <cmcc...@apache.org> >> wrote: >> >>> >> >>>> On Mon, Jul 23, 2018, at 23:20, James Cheng wrote: >> >>>>> Hi Stanislav! Thanks for this KIP! >> >>>>> >> >>>>> I agree that it would be good if the LogCleaner were more tolerant >> of >> >>>>> errors. Currently, as you said, once it dies, it stays dead. >> >>>>> >> >>>>> Things are better now than they used to be. We have the metric >> >>>>> kafka.log:type=LogCleanerManager,name=time-since-last-run-ms >> >>>>> which we can use to tell us if the threads are dead. And as of >> 1.1.0, >> >>> we >> >>>>> have KIP-226, which allows you to restart the log cleaner thread, >> >>>>> without requiring a broker restart. >> >>>>> >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-226+-+Dynamic+Broker+Configuration >> >>>>> < >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-226+-+Dynamic+Broker+Configuration >> >>>> >> >>>>> I've only read about this, I haven't personally tried it. >> >>>> Thanks for pointing this out, James! Stanislav, we should probably >> >> add a >> >>>> sentence or two mentioning the KIP-226 changes somewhere in the KIP. >> >>> Maybe >> >>>> in the intro section? >> >>>> >> >>>> I think it's clear that requiring the users to manually restart the >> log >> >>>> cleaner is not a very good solution. But it's good to know that >> it's a >> >>>> possibility on some older releases. >> >>>> >> >>>>> Some comments: >> >>>>> * I like the idea of having the log cleaner continue to clean as >> many >> >>>>> partitions as it can, skipping over the problematic ones if >> possible. >> >>>>> >> >>>>> * If the log cleaner thread dies, I think it should automatically be >> >>>>> revived. Your KIP attempts to do that by catching exceptions during >> >>>>> execution, but I think we should go all the way and make sure that a >> >>> new >> >>>>> one gets created, if the thread ever dies. >> >>>> This is inconsistent with the way the rest of Kafka works. We don't >> >>>> automatically re-create other threads in the broker if they >> terminate. >> >>> In >> >>>> general, if there is a serious bug in the code, respawning threads is >> >>>> likely to make things worse, by putting you in an infinite loop which >> >>>> consumes resources and fires off continuous log messages. >> >>>> >> >>>>> * It might be worth trying to re-clean the uncleanable partitions. >> >> I've >> >>>>> seen cases where an uncleanable partition later became cleanable. I >> >>>>> unfortunately don't remember how that happened, but I remember being >> >>>>> surprised when I discovered it. It might have been something like a >> >>>>> follower was uncleanable but after a leader election happened, the >> >> log >> >>>>> truncated and it was then cleanable again. I'm not sure. >> >>>> James, I disagree. We had this behavior in the Hadoop Distributed >> File >> >>>> System (HDFS) and it was a constant source of user problems. >> >>>> >> >>>> What would happen is disks would just go bad over time. The DataNode >> >>>> would notice this and take them offline. But then, due to some >> >>>> "optimistic" code, the DataNode would periodically try to re-add them >> >> to >> >>>> the system. Then one of two things would happen: the disk would just >> >>> fail >> >>>> immediately again, or it would appear to work and then fail after a >> >> short >> >>>> amount of time. >> >>>> >> >>>> The way the disk failed was normally having an I/O request take a >> >> really >> >>>> long time and time out. So a bunch of request handler threads would >> >>>> basically slam into a brick wall when they tried to access the bad >> >> disk, >> >>>> slowing the DataNode to a crawl. It was even worse in the second >> >>> scenario, >> >>>> if the disk appeared to work for a while, but then failed. Any data >> >> that >> >>>> had been written on that DataNode to that disk would be lost, and we >> >>> would >> >>>> need to re-replicate it. >> >>>> >> >>>> Disks aren't biological systems-- they don't heal over time. Once >> >>> they're >> >>>> bad, they stay bad. The log cleaner needs to be robust against cases >> >>> where >> >>>> the disk really is failing, and really is returning bad data or >> timing >> >>> out. >> >>>>> * For your metrics, can you spell out the full metric in JMX-style >> >>>>> format, such as: >> >>>>> >> >>> kafka.log:type=LogCleanerManager,name=uncleanable-partitions-count >> >>>>> value=4 >> >>>>> >> >>>>> * For "uncleanable-partitions": topic-partition names can be very >> >> long. >> >>>>> I think the current max size is 210 characters (or maybe 240-ish?). >> >>>>> Having the "uncleanable-partitions" being a list could be very large >> >>>>> metric. Also, having the metric come out as a csv might be difficult >> >> to >> >>>>> work with for monitoring systems. If we *did* want the topic names >> to >> >>> be >> >>>>> accessible, what do you think of having the >> >>>>> kafka.log:type=LogCleanerManager,topic=topic1,partition=2 >> >>>>> I'm not sure if LogCleanerManager is the right type, but my example >> >> was >> >>>>> that the topic and partition can be tags in the metric. That will >> >> allow >> >>>>> monitoring systems to more easily slice and dice the metric. I'm not >> >>>>> sure what the attribute for that metric would be. Maybe something >> >> like >> >>>>> "uncleaned bytes" for that topic-partition? Or >> time-since-last-clean? >> >>> Or >> >>>>> maybe even just "Value=1". >> >>>> I haven't though about this that hard, but do we really need the >> >>>> uncleanable topic names to be accessible through a metric? It seems >> >> like >> >>>> the admin should notice that uncleanable partitions are present, and >> >> then >> >>>> check the logs? >> >>>> >> >>>>> * About `max.uncleanable.partitions`, you said that this likely >> >>>>> indicates that the disk is having problems. I'm not sure that is the >> >>>>> case. For the 4 JIRAs that you mentioned about log cleaner problems, >> >>> all >> >>>>> of them are partition-level scenarios that happened during normal >> >>>>> operation. None of them were indicative of disk problems. >> >>>> I don't think this is a meaningful comparison. In general, we don't >> >>>> accept JIRAs for hard disk problems that happen on a particular >> >> cluster. >> >>>> If someone opened a JIRA that said "my hard disk is having problems" >> we >> >>>> could close that as "not a Kafka bug." This doesn't prove that disk >> >>>> problems don't happen, but just that JIRA isn't the right place for >> >>> them. >> >>>> I do agree that the log cleaner has had a significant number of logic >> >>>> bugs, and that we need to be careful to limit their impact. That's >> one >> >>>> reason why I think that a threshold of "number of uncleanable logs" >> is >> >> a >> >>>> good idea, rather than just failing after one IOException. In all >> the >> >>>> cases I've seen where a user hit a logic bug in the log cleaner, it >> was >> >>>> just one partition that had the issue. We also should increase test >> >>>> coverage for the log cleaner. >> >>>> >> >>>>> * About marking disks as offline when exceeding a certain threshold, >> >>>>> that actually increases the blast radius of log compaction failures. >> >>>>> Currently, the uncleaned partitions are still readable and writable. >> >>>>> Taking the disks offline would impact availability of the >> uncleanable >> >>>>> partitions, as well as impact all other partitions that are on the >> >>> disk. >> >>>> In general, when we encounter I/O errors, we take the disk partition >> >>>> offline. This is spelled out in KIP-112 ( >> >>>> >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-112%3A+Handle+disk+failure+for+JBOD >> >>>> ) : >> >>>> >> >>>>> - Broker assumes a log directory to be good after it starts, and >> mark >> >>>> log directory as >> >>>>> bad once there is IOException when broker attempts to access (i.e. >> >> read >> >>>> or write) the log directory. >> >>>>> - Broker will be offline if all log directories are bad. >> >>>>> - Broker will stop serving replicas in any bad log directory. New >> >>>> replicas will only be created >> >>>>> on good log directory. >> >>>> The behavior Stanislav is proposing for the log cleaner is actually >> >> more >> >>>> optimistic than what we do for regular broker I/O, since we will >> >> tolerate >> >>>> multiple IOExceptions, not just one. But it's generally consistent. >> >>>> Ignoring errors is not. In any case, if you want to tolerate an >> >>> unlimited >> >>>> number of I/O errors, you can just set the threshold to an infinite >> >> value >> >>>> (although I think that would be a bad idea). >> >>>> >> >>>> best, >> >>>> Colin >> >>>> >> >>>>> -James >> >>>>> >> >>>>> >> >>>>>> On Jul 23, 2018, at 5:46 PM, Stanislav Kozlovski < >> >>>> stanis...@confluent.io> wrote: >> >>>>>> I renamed the KIP and that changed the link. Sorry about that. Here >> >>> is >> >>>> the >> >>>>>> new link: >> >>>>>> >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-346+-+Improve+LogCleaner+behavior+on+error >> >>>>>> On Mon, Jul 23, 2018 at 5:11 PM Stanislav Kozlovski < >> >>>> stanis...@confluent.io> >> >>>>>> wrote: >> >>>>>> >> >>>>>>> Hey group, >> >>>>>>> >> >>>>>>> I created a new KIP about making log compaction more >> >> fault-tolerant. >> >>>>>>> Please give it a look here and please share what you think, >> >>>> especially in >> >>>>>>> regards to the points in the "Needs Discussion" paragraph. >> >>>>>>> >> >>>>>>> KIP: KIP-346 >> >>>>>>> < >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-346+-+Limit+blast+radius+of+log+compaction+failure >> >>>>>>> -- >> >>>>>>> Best, >> >>>>>>> Stanislav >> >>>>>>> >> >>>>>> >> >>>>>> -- >> >>>>>> Best, >> >>>>>> Stanislav >> > >> >> > > -- > Best, > Stanislav > -- Best, Stanislav
