[ https://issues.apache.org/jira/browse/CASSANDRA-15364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16957064#comment-16957064 ]
Marcus Eriksson commented on CASSANDRA-15364: --------------------------------------------- bq. As a general comment, almost every case I see where absolutePath.ifPresent is called is known to be a ADD or REMOVE guess I could add asserts that it exists instead, would probably be clearer bq. Is this to handle failure in the middle of the log (while deleting on commit we hard-stop in the middle, so recovery may see missing files)? the null check is unnecessary, I'll remove it bq. Took a while to parse this change, so I am curious what was the motivation? I was mostly worried that this patch would slow things down in the "normal case" (a tiny number of remove records in a directory with many files) by parsing the filename and creating a Descriptor for every file in the directory when we really don't have to. > Avoid over scanning data directories in LogFile.verify() > -------------------------------------------------------- > > Key: CASSANDRA-15364 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15364 > Project: Cassandra > Issue Type: Bug > Components: Local/Compaction > Reporter: Marcus Eriksson > Assignee: Marcus Eriksson > Priority: Normal > Fix For: 3.0.x, 3.11.x, 4.x > > > We currently list the data directory for every {{REMOVE}} record in the file > in {{LogFile.verify()}} - this can get very expensive during startup when we > call {{LogTransaction.removeUnfinishedLeftovers()}}. In > {{LogRecord.getExistingFiles(Set<String> absoluteFilePaths)}} we also fully > parse the file name of the sstables found, here we only need to prefix match. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org