Skip to content

Commit

Permalink
Make sure repair interval is never shorter than initial delay
Browse files Browse the repository at this point in the history
  • Loading branch information
jwaeab committed May 22, 2024
1 parent d04638b commit 2ef0603
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Changes
## Version 5.0.4

* ecChronos will break if repair interval is shorter than the initial delay - Issue #667

## Version 5.0.3

* Spring Framework URL Parsing with Host Validation is vulnerable - Issue #661
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,24 @@ public final long getInterval(final TimeUnit timeUnit)
return timeUnit.convert(myTime, myUnit);
}

@JsonProperty("time")
public final long getTime()
{
return myTime;
}

@JsonProperty("time")
public final void setTime(final long time)
{
myTime = time;
}

@JsonProperty("unit")
public final TimeUnit getUnit()
{
return myUnit;
}

@JsonProperty("unit")
public final void setUnit(final String unit)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@
import com.ericsson.bss.cassandra.ecchronos.core.repair.RepairOptions;
import com.ericsson.bss.cassandra.ecchronos.core.utils.UnitConverter;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class RepairConfig
{
private static final Logger LOG = LoggerFactory.getLogger(RepairConfig.class);

private static final int DAYS_INTERVAL = 7;
private static final int DAYS_WARNING = 8;
private static final int DAYS_ERROR = 10;
Expand Down Expand Up @@ -55,6 +59,12 @@ public final void setPriority(final Priority priority)
myPriority = priority;
}

@JsonProperty("interval")
public final Interval getRepairInterval()
{
return myRepairInterval;
}

@JsonProperty("interval")
public final void setRepairInterval(final Interval repairInterval)
{
Expand Down Expand Up @@ -140,13 +150,13 @@ public final void validate(final String repairConfigType)
repairIntervalSeconds, warningIntervalSeconds));
}

if (initialDelaySeconds >= repairIntervalSeconds)
if (repairIntervalSeconds < initialDelaySeconds)
{
throw new IllegalArgumentException(String.format(
"%s initial delay must be shorter than the repair interval."
+ " Repair interval: %d seconds, initial delay : %d seconds", repairConfigType,
repairIntervalSeconds, initialDelaySeconds));
LOG.warn("{} repair interval ({}s) is shorter than initial delay ({}s). Will use {}s as initial delay.",
repairConfigType, repairIntervalSeconds, initialDelaySeconds, repairIntervalSeconds);
myInitialDelay = new Interval(myRepairInterval.getTime(), myRepairInterval.getUnit());
}

long errorIntervalSeconds = myAlarm.getErrorInterval().getInterval(TimeUnit.SECONDS);
if (warningIntervalSeconds >= errorIntervalSeconds)
{
Expand Down
4 changes: 2 additions & 2 deletions application/src/main/resources/ecc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ repair:
time: 7
unit: days
##
## Initial delay for new tables. New tables are always assumed to have been repaired in the past, however a delay
## can be set for the first repair. This will not affect subsequent repairs and defaults to one day.
## Initial delay for new tables. New tables are always assumed to have been repaired in the past by the interval.
## However, a delay can be set for the first repair. This will not affect subsequent repairs and defaults to one day.
##
initial_delay:
time: 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,13 @@ public void testInitialDelayLongerThanRepairInterval() throws Exception
File file = new File(classLoader.getResource("initial_delay_greater_than_repair.yml").getFile());

ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory());
assertThatExceptionOfType(JsonMappingException.class).isThrownBy(() -> objectMapper.readValue(file, Config.class));
Config config = objectMapper.readValue(file, Config.class);
GlobalRepairConfig repair = config.getRepairConfig();
Interval repint = repair.getRepairInterval();
Interval intdel = repair.getInitialDelay();

assertThat(repint.getTime()).isEqualTo(intdel.getTime());
assertThat(repint.getUnit()).isEqualTo(intdel.getUnit());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,9 @@ private long newTableRepairedAt()
long initialDelayInMs = myRepairConfiguration.getInitialDelayInMs();
long assumedRepairedAt = System.currentTimeMillis() - runIntervalInMs + initialDelayInMs;

if (LOG.isInfoEnabled())
{
LOG.info("Assuming the table {} is new. Next repair will occur at {}",
myTableReference, MY_DATE_FORMAT.get().format(new Date(assumedRepairedAt + runIntervalInMs)));
}
LOG.info("Assuming the table {} is new. Next repair will occur at {}.",
myTableReference,
MY_DATE_FORMAT.get().format(new Date(assumedRepairedAt + runIntervalInMs)));

return assumedRepairedAt;
}
Expand Down
6 changes: 3 additions & 3 deletions docs/GETTING_STARTED.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ Installation information can be found in [SETUP.md](SETUP.md).

When ecChronos is started, it will read repair history to see the status of the repairs.
If repair history exists for a table, ecChronos will use the last time repair was run and add the repair interval to calculate the next repair time.
On the other hand, if there's no repair history present for a table, the ecChronos will assume that the table has been repaired in the past using
initial delay to calculate.
On the other hand, if there's no repair history present for a table, ecChronos will assume the table has been repaired in the past, but an initial delay (defaults to one day) will then offset the first repair.
If the repair interval is shorter than the configured initial delay, initial delay will be set to the same value as the repair interval.

The assumption is done in the following way:
* Initial delay is configurable (default 1 day)
* Initial delay = min(initial delay, repair interval)
* Completed at = (start time - repair interval + initial delay)
* Next repair = (completed at + repair interval)

Expand Down
4 changes: 2 additions & 2 deletions docs/autogenerated/EccYamlFile.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@
* time: 7
* unit: days
#
# Initial delay for new tables. New tables are always assumed to have been repaired in the past, however a delay
# can be set for the first repair. This will not affect subsequent repairs and defaults to one day.
# Initial delay for new tables. New tables are always assumed to have been repaired in the past by the interval.
# However, a delay can be set for the first repair. This will not affect subsequent repairs and defaults to one day.
#
**initial_delay:**
* time: 1
Expand Down

0 comments on commit 2ef0603

Please sign in to comment.