Skip to content
Snippets Groups Projects

Bulk updates

Merged Bulk updates
bulk_updates into master
Merged Ghost User requested to merge bulk_updates into master

Pending:

  • renaming (update vs. request vs. query ...)
  • proper logging (waiting for !250 (merged))
  • rrset_is_nsec3_rel() and mm_realloc copypasta
  • partial zone adjustment - next MR

Pros:

  • faster for lots of small updates
  • improved terrible empty node cleanup (now takes time proportional to size of update, not size of the zone)
  • spinlocks instead of mutexes for ddns q access (faster for this case + less memory :fisted_hand_sign: )

Cons:

  • if one transfer fails (no matter the reason, except for acl issues), they all fail
  • might actually be slower for small singular updates, but oh well

Not sure if good or bad:

  • most of logging is lost (only logs errors in detail, i.e. who sent the faulty update)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/ddns.c
    881 882
    882 883 /* ---------------------------------- API ----------------------------------- */
    883 884
    884 int ddns_process_prereqs(const knot_pkt_t *query, const zone_contents_t *zone,
    885 int ddns_process_prereqs(const knot_pkt_t *query, zone_update_t *update,
    885 886 uint16_t *rcode)
    • Jan Včelák
      Jan Včelák @jvcelak ·
      Contributor

      What about changing type of rcode variable to knot_rcode_t instead of unt16_t?

      The benefit of that would be at least some type control of passed values.

    • Ghost User
      Ghost User @ghost ·
      Author Contributor

      I think we were removing knot_rcode_t from lot of places, but I don't remember why. Maybe because knot_rcode_t is an enum, therefore int, and it might mess up wire conversion functions.

    • Jan Včelák
      Jan Včelák @jvcelak ·
      Contributor

      I think it should not mess the conversion, because the conversion between enum and uint16_t will happen at the call time of a wire_write_* function.

      But nevermind. It's not important.

  • Jan Včelák
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/nameserver/update.c
    161 zone_update_t zone_update;
    162 zone_update_init(&zone_update, zone->contents, &ddns_ch);
    163
    164 // Walk all the requests and process.
    165 struct request_data *req;
    166 WALK_LIST(req, *requests) {
    167 ret = process_single_update(req, zone, &zone_update);
    168 if (ret != KNOT_EOK) {
    169 changeset_clear(&ddns_ch);
    170 set_rcodes(requests, KNOT_RCODE_SERVFAIL);
    171 return ret;
    172 }
    173 }
    152 174
    153 175 zone_contents_t *new_contents = NULL;
    154 176 const bool change_made = !changeset_empty(&ddns_ch);
    • Jan Včelák
      Jan Včelák @jvcelak ·
      Contributor

      Code around here could be a little reformated to decrease necessary indentation:

      if (changeset_empty(&ddns_ch)) {
    changeset_clear(&ddns_ch);
    return KNOT_EOK;
}

// ... changeset application ...

      But this is acceptable. :-)

    • Ghost User
      Ghost User @ghost ·
      Author Contributor

      But what about my branch predictors :( ? just joking, :white_check_mark:

  • Jan Včelák
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/nameserver/update.c
    276 289 struct requestor re;
    277 requestor_init(&re, NS_PROC_CAPTURE, qdata->mm);
    290 requestor_init(&re, NS_PROC_CAPTURE, NULL);
    278 291
    279 292 /* Fetch primary master. */
    280 const conf_iface_t *master = zone_master(qdata->zone);
    293 const conf_iface_t *master = zone_master(zone);
    281 294
    282 295 /* Copy request and assign new ID. */
    283 knot_pkt_t *query = knot_pkt_new(NULL, pkt->max_size, qdata->mm);
    284 int ret = knot_pkt_copy(query, qdata->query);
    296 knot_pkt_t *query = knot_pkt_new(NULL, request->query->max_size, NULL);
    297 int ret = knot_pkt_copy(query, request->query);
    285 298 if (ret != KNOT_EOK) {
    299 knot_wire_set_rcode(request->resp->wire, KNOT_RCODE_SERVFAIL);
    286 300 return ret;
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/apply.c
    63 64 return KNOT_EOK;
    64 65 }
    65 66
    67 /* ------------------------- Empty node cleanup ----------------------------- */
    68
    69 /*! \todo move this to new zone API - zone should do this automatically. */
    70 /*! \brief Deletes possibly empty node and all its empty parents recursively. */
    71 static void delete_empty_node(zone_tree_t *tree, zone_node_t *node)
    72 {
    73 if (node->rrset_count == 0 && node->children == 0) {
  • Jan Včelák
  • Jan Včelák
  • Jan Včelák
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/ddns.c
    904 905 }
    905 906
    906 907 // Check stored RRSets
    907 ret = check_stored_rrsets(&rrset_list, zone, rcode);
    908 ret = check_stored_rrsets(&rrset_list, update, rcode);
    908 909 rrset_list_clear(&rrset_list);
    909 910 return ret;
    910 911 }
    911 912
    912 913 int ddns_process_update(const zone_t *zone, const knot_pkt_t *query,
    913 changeset_t *changeset, uint16_t *rcode)
    914 zone_update_t *update, uint16_t *rcode)
  • Jan Včelák
  • Jan Včelák
  • Jan Včelák
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/zone-update.c
    114 int ret = deep_copy_node_data(synth_node, old_node, mm);
    115 if (ret != KNOT_EOK) {
    116 node_free(&synth_node, mm);
    117 return NULL;
    118 }
    119
    120 return synth_node;
    121 }
    122
    123 /* ------------------------------- API -------------------------------------- */
    124
    125 void zone_update_init(zone_update_t *update, const zone_contents_t *zone, changeset_t *change)
    126 {
    127 update->zone = zone;
    128 update->change = change;
    129 mm_ctx_mempool(&update->mm, 4096);
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/zone-update.c
    157 old_node = add_node;
    158 add_node = NULL;
    159 }
    160 }
    161
    162 // We have to apply changes to node.
    163 zone_node_t *synth_node = node_deep_copy(old_node, &update->mm);
    164 if (synth_node == NULL) {
    165 return NULL;
    166 }
    167
    168 // Apply changes to node.
    169 int ret = apply_changes_to_node(synth_node, add_node, rem_node,
    170 &update->mm);
    171 if (ret != KNOT_EOK) {
    172 return NULL;
  • Jan Včelák
  • Jan Včelák
    Jan Včelák @jvcelak started a thread on the diff
    • src/knot/updates/zone-update.h
    1 /*!
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
    • Please register or sign in to reply