Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cassandra's pool duration mapping uses a wrong duration unit #23249

Closed
littleatp opened this issue Sep 11, 2020 · 2 comments
Closed

Cassandra's pool duration mapping uses a wrong duration unit #23249

littleatp opened this issue Sep 11, 2020 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@littleatp
Copy link

I use the spring-boot-data-cassandra to connect to my cassandra cluster.
my application configuration is so:

spring.data.cassandra.pool.heartbeat-interval=30s

as above, I set the heartbeat-interval to 30 seconds.
but after application started, I cann't see any debug information about heartbeat actions by datastax driver.
my log4j configuration was set to :

 <Logger name="org.springframework.data.cassandra" level="debug" additivity="true">
        </Logger>
        <Logger name="com.datastax" level="debug" additivity="true">
</Logger>

Then I turn on the code, found it may be a bug by CassandraAutoConfiguration, the snippets as so:

private void mapPoolingOptions(CassandraProperties properties, CassandraDriverOptions options) {
		PropertyMapper map = PropertyMapper.get();
		CassandraProperties.Pool poolProperties = properties.getPool();
		map.from(poolProperties::getIdleTimeout).whenNonNull().asInt(Duration::getSeconds)
				.to((idleTimeout) -> options.add(DefaultDriverOption.HEARTBEAT_TIMEOUT, idleTimeout));
		map.from(poolProperties::getHeartbeatInterval).whenNonNull().asInt(Duration::getSeconds)
				.to((heartBeatInterval) -> options.add(DefaultDriverOption.HEARTBEAT_INTERVAL, heartBeatInterval));
	}

as above, it set DefaultDriverOption.HEARTBEAT_INTERVAL to an integer value(seconds),but datastax driver treat it as mills default, just like this:
com.typesafe.config.impl.SimpleConfig.java

    public static long parseDuration(String input,
            ConfigOrigin originForException, String pathForException) {
        String s = ConfigImplUtil.unicodeTrim(input);
        String originalUnitString = getUnits(s);
        String unitString = originalUnitString;
        String numberString = ConfigImplUtil.unicodeTrim(s.substring(0, s.length()
                - unitString.length()));
        TimeUnit units = null;

        // this would be caught later anyway, but the error message
        // is more helpful if we check it here.
        if (numberString.length() == 0)
            throw new ConfigException.BadValue(originForException,
                    pathForException, "No number in duration value '" + input
                            + "'");

        if (unitString.length() > 2 && !unitString.endsWith("s"))
            unitString = unitString + "s";

        // note that this is deliberately case-sensitive
        if (unitString.equals("") || unitString.equals("ms") || unitString.equals("millis")
                || unitString.equals("milliseconds")) {
            units = TimeUnit.MILLISECONDS;

so, because 30 millis was too short, and HeartbeatHandler cast it to 0 seconds
com.datastax.oss.driver.internal.core.channel.HeartbeatHandler:

  HeartbeatHandler(DriverExecutionProfile config) {
    super((int) config.getDuration(DefaultDriverOption.HEARTBEAT_INTERVAL).getSeconds(), 0, 0);
    this.config = config;
  }

so, there may be some thing to change about CassandraAutoConfiguration ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 11, 2020
@snicoll snicoll changed the title The CassandraAutoConfiguration give the wrong heartbeat parameters. Cassandra's heartbeat interval mapping uses a wrong duration unit Sep 11, 2020
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 11, 2020
@snicoll snicoll added this to the 2.3.x milestone Sep 11, 2020
@snicoll snicoll self-assigned this Sep 11, 2020
@snicoll
Copy link
Member

snicoll commented Sep 11, 2020

@littleatp thanks for the report. You are right, it is using the wrong unit. I got tricked by the fact that the driver itself was requesting seconds from the configuration. The Heartbeat timeout is most probably affected as well.

@snicoll
Copy link
Member

snicoll commented Sep 11, 2020

Given SimpleConfig#parseDuration contract, we should be indeed using ms for all keys. This also means that the default unit for the heartbeat properties is wrong.

@snicoll snicoll changed the title Cassandra's heartbeat interval mapping uses a wrong duration unit Cassandra's heartbeat duration mapping uses a wrong duration unit Sep 11, 2020
@snicoll snicoll modified the milestones: 2.3.x, 2.3.4 Sep 11, 2020
@snicoll snicoll changed the title Cassandra's heartbeat duration mapping uses a wrong duration unit Cassandra's pool duration mapping uses a wrong duration unit Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants