Skip to content

poller_push_data_to_main() drops every 501st host and every 10001st poller_item #602

Description

@somethingwithproof

poller_push_data_to_main() batches rows into sqlbuf and flushes when the batch is full, but the flush happens instead of emitting the row that triggered it, so that row never reaches the main server.

util.c:1036 (develop) / util.c:907 (1.2.x):

while ((row = mysql_fetch_row(result))) {
    if (rows < 500) {
        ...build the row...
        rows++;
    } else {
        sqlp += snprintf(sqlp, remaining, "%s", suffix);
        db_insert(&mysqlr, REMOTE, sqlbuf);

        rows = 0;          /* the current row is never emitted */
    }
}

When rows reaches 500 the current row falls into the else, the batch is sent, rows resets, and the loop moves to the next row. The device that landed on the boundary is dropped. The poller_item loop at util.c:1160 / util.c:1004 has the same shape with a cap of 10000.

So a remote poller with more than 500 devices silently omits every 501st device's status from each push, and one with more than 10000 poller items omits every 10001st item's rrd_next_step. The count is exact and repeats every cycle, and since the batch itself succeeds there is nothing in the log.

Separately, the row count is not a size bound. sqlbuf is HUGE_BUFSIZE (2,048,000 bytes). Six of the interpolated host columns are device-supplied SNMP strings escaped into tmpstr[DBL_BUFSIZE], so each contributes up to 2047 bytes. Several KB per row times 500 rows exceeds the buffer. On develop the writes go through the sqlp += snprintf(...) idiom, which walks the cursor past the end of the buffer on truncation (#588), so this is also how that idiom is reached in practice.

Suggested fix is to flush before the row rather than in place of it, and to test remaining bytes as well as the row count:

while ((row = mysql_fetch_row(result))) {
    remaining = HUGE_BUFSIZE - (sqlp - sqlbuf);

    if (rows > 0 && (rows >= 500 || remaining < PUSH_ROW_MAX)) {
        flush();
        rows = 0;
    }

    ...build the row...
    rows++;
}

with the flush refusing to send a statement that reported a truncation rather than shipping one cut inside a quoted value.

Present on develop and 1.2.x. A fix and structural regression tests are in #597.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions