jmckenzie-dev commented on code in PR #179:
URL:
https://github.com/apache/cassandra-analytics/pull/179#discussion_r2918523981
##########
cassandra-analytics-cdc/src/test/java/org/apache/cassandra/cdc/LocalCommitLog.java:
##########
@@ -67,7 +70,21 @@ public long length()
public boolean completed()
{
// for CDC Cassandra writes COMPLETED in the final line of the
CommitLog-7-*_cdc.idx index file.
- return maxOffset() >= 67108818;
+ List<String> lines = readIdxFile();
+ return lines.size() >= 2 &&
lines.get(1).trim().equalsIgnoreCase("COMPLETED");
+ }
+
+ private List<String> readIdxFile()
+ {
+ Path idxPath = path.resolveSibling(name.replace(".log", "_cdc.idx"));
Review Comment:
Recommend adding a brief comment here that each log file has a corresponding
`_cdc.idx` file that goes along with it; ironically I was the one that wrote
the patch that made that change and I couldn't remember off the top of my head
the relationship between the two. :)
A follow up JIRA for C* proper or the sidecar to provide interfaces that
give this functionality around CDC (get the file that goes with commitlog X,
tell me if it's COMPLETED, etc) seems like a useful primitive abstraction for
us to extract. Maybe that's what `LocalCommitLog.java` and `CommitLog.java` are
intended to be in the analytics library here? The names aren't great IMO and
the overlap with what's in the core C* database make that more confusing to me.
Since none of these classes have top level javadocs explaining their purpose
nor their relationship to the root classes and concepts in the core C*
database, it leaves a lot up to the interpretation of other maintainers that
come to this code cold.
So for this patch: brief comment here, and I'll actually take the note for
the follow up JIRA for the above if that resonates with you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
