[ 
https://issues.apache.org/jira/browse/CASSANDRA-14373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16639846#comment-16639846
 ] 

Marcus Eriksson commented on CASSANDRA-14373:
---------------------------------------------

thanks for the review, pushed a couple of commits to address the comments

bq. Removing documentation of defaults doesn't seem like a pure win since they 
still seem to be there?
I removed the defaults from the nodetool command to be able to override options 
that are set in cassandra.yaml, do you have a suggestion how to do it in a 
better way? We don't know the actual set defaults in the nodetool command, the 
user would have to check out cassandra.yaml which might not be very user 
friendly.
bq. This isn't just a path right it's a format specified of sorts with %path?
right, changed to 'command' instead
bq. This is BinLogOptions but the comments reference Audit log and there is a 
typo in the first sentence here
fixed
bq. Depending on what the archiving script does and why it failed there could 
be unfortunate consequences to retrying repeatedly.
added a new configuration param that allows users to set max retries to 0 to 
avoid this (defaults to 10 retries as retrying forever might also be bad)
bq. exec forks, forking can be slow because of page table copying which in the 
past was slow under things like Xen.. I'm just mentioning it. I don't think you 
need to make it better right now. I don't know offhand how you invoke an 
external command more efficiently from Java.
yeah not sure what to do here, a quick search tells me ProcessBuilder seems to 
be the way to do this
bq. Is this going to enable it for all tests? Is that a good idea can we only 
enable it for just the unit tests that require it?
yeah, removed, should not be there
bq. Should use execute instead of submit unless consuming the result future
fixed
bq. Same here
here we actually wait for the future

bq. The dtests are good tests but could they be unit tests since they are 
single node. 
In general I agree, but in this case it executes the nodetool command as an end 
user would, against a running cassandra cluster (well, node, but anyway). I 
suppose we could stand up a real cluster in a unit test and execute the 
nodetool script as well, but I assume that would take about as long as doing it 
using ccm.

> Allow using custom script for chronicle queue BinLog archival
> -------------------------------------------------------------
>
>                 Key: CASSANDRA-14373
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14373
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Stefan Podkowinski
>            Assignee: Pramod K Sivaraju
>            Priority: Major
>              Labels: lhf, pull-request-available
>             Fix For: 4.x
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> It would be nice to allow the user to configure an archival script that will 
> be executed in {{BinLog.onReleased(cycle, file)}} for every deleted bin log, 
> just as we do in {{CommitLogArchiver}}. The script should be able to copy the 
> released file to an external location or do whatever the author hand in mind. 
> Deleting the log file should be delegated to the script as well.
> See CASSANDRA-13983, CASSANDRA-12151 for use cases.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to