> On March 26, 2014, 12:55 a.m., Neha Narkhede wrote: > > My guess is that we may have to review and make a similar change in the > > following places - > > 1. LogManager.checkpointRecoveryPointOffsets() > > 2. LogManager.nextLogDir() > > > > The unit test in ReplicaManagerTest cover checkpointing, but we also need > > to add coverage for log recovery and log cleaning since those are the > > places that touch the directory mapping > > Timothy Chen wrote: > I just looked at the LogManager and looks like they have their own > internal directory mapping that uses File objects, and LogManager constructor > only accepts array of File directories so I don't think relative/absolute or > different path trailing conventions has much meaning to be tested for LogMgr. > Thoughts? > > Neha Narkhede wrote: > hmm.. checkpointRecoveryPointOffsets() also seemed to access the parent > directory - > > val recoveryPointsByDir = > this.logsByTopicPartition.groupBy(_._2.dir.getParent.toString) > > Same in nextLogDir() - > > val logCounts = allLogs.groupBy(_.dir.getParent).mapValues(_.size) > val zeros = logDirs.map(dir => (dir.getPath, 0)).toMap > > I'm not sure that this will necessarily be a problem, but it will be good > to get unit test coverage for this bug for the above 2 APIs. In general, that > would be log recovery and log cleaning.
I think the difference really is that in ReplicaManagerTest we read in configuration log.dirs directly as string and store them in the map keys, where here they're all wrapped with File object so paths have the same convention when they are returned. I will add Log manager tests just to verify this. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19626/#review38565 ----------------------------------------------------------- On March 25, 2014, 6:54 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19626/ > ----------------------------------------------------------- > > (Updated March 25, 2014, 6:54 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1323 > https://issues.apache.org/jira/browse/KAFKA-1323 > > > Repository: kafka > > > Description > ------- > > KAFKA-1323 Fix log directory to support relative directories > > > Diffs > ----- > > core/src/main/scala/kafka/cluster/Partition.scala > 0b88f14c4855b27242906cd45930bae501e26226 > core/src/main/scala/kafka/server/ReplicaManager.scala > 255be063ee247a849e527297c987da4625d749ca > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala > b5936d4101b513baa805ab26361fe965bdf980aa > > Diff: https://reviews.apache.org/r/19626/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
