Check operation sequencing and stop clients when it breaks

src/client/cluster_client.cpp

@@ -1244,7 +1244,6 @@ int cluster_client_t::try_send(cluster_op_t *op, int i)
                 .req = { .rw = {
                     .header = {
                         .magic = SECONDARY_OSD_OP_MAGIC,
-                        .id = next_op_id(),
                         .opcode = op->opcode == OSD_OP_READ_BITMAP || op->opcode == OSD_OP_READ_CHAIN_BITMAP ? OSD_OP_READ : op->opcode,
                     },
                     .inode = op->cur_inode,
@@ -1353,7 +1352,6 @@ void cluster_client_t::send_sync(cluster_op_t *op, cluster_op_part_t *part)
         .req = {
             .hdr = {
                 .magic = SECONDARY_OSD_OP_MAGIC,
-                .id = next_op_id(),
                 .opcode = OSD_OP_SYNC,
             },
         },
@@ -1498,8 +1496,3 @@ void cluster_client_t::copy_part_bitmap(cluster_op_t *op, cluster_op_part_t *par
         part_len--;
     }
 }
-
-uint64_t cluster_client_t::next_op_id()
-{
-    return msgr.next_subop_id++;
-}

src/client/cluster_client.h

@@ -152,7 +152,6 @@ public:
 
     //inline uint32_t get_bs_bitmap_granularity() { return st_cli.global_bitmap_granularity; }
     //inline uint64_t get_bs_block_size() { return st_cli.global_block_size; }
-    uint64_t next_op_id();
 
 #ifndef __MOCK__
 protected:

src/client/cluster_client_list.cpp

@@ -342,7 +342,6 @@ void cluster_client_t::send_list(inode_list_osd_t *cur_list)
         .sec_list = {
             .header = {
                 .magic = SECONDARY_OSD_OP_MAGIC,
-                .id = next_op_id(),
                 .opcode = OSD_OP_SEC_LIST,
             },
             .list_pg = cur_list->pg->pg_num,

src/client/messenger.cpp

@@ -217,7 +217,6 @@ void osd_messenger_t::init()
                     op->req = (osd_any_op_t){
                         .hdr = {
                             .magic = SECONDARY_OSD_OP_MAGIC,
-                            .id = this->next_subop_id++,
                             .opcode = OSD_OP_PING,
                         },
                     };
@@ -629,11 +628,17 @@ void osd_messenger_t::check_peer_config(osd_client_t *cl)
         .show_conf = {
             .header = {
                 .magic = SECONDARY_OSD_OP_MAGIC,
-                .id = this->next_subop_id++,
                 .opcode = OSD_OP_SHOW_CONFIG,
             },
         },
     };
+    json11::Json::object payload;
+    if (osd_num)
+    {
+        // Inform that we're OSD <osd_num>
+        payload["osd_num"] = osd_num;
+    }
+    payload["check_sequencing"] = true;
 #ifdef WITH_RDMA
     if (!use_rdmacm && rdma_contexts.size())
     {
@@ -649,19 +654,20 @@ void osd_messenger_t::check_peer_config(osd_client_t *cl)
             cl->rdma_conn = msgr_rdma_connection_t::create(selected_ctx, rdma_max_send, rdma_max_recv, rdma_max_sge, rdma_max_msg);
             if (cl->rdma_conn)
             {
-                json11::Json payload = json11::Json::object {
+                payload["connect_rdma"] = cl->rdma_conn->addr.to_string();
-                    { "connect_rdma", cl->rdma_conn->addr.to_string() },
+                payload["rdma_max_msg"] = cl->rdma_conn->max_msg;
-                    { "rdma_max_msg", cl->rdma_conn->max_msg },
-                };
-                std::string payload_str = payload.dump();
-                op->req.show_conf.json_len = payload_str.size();
-                op->buf = malloc_or_die(payload_str.size());
-                op->iov.push_back(op->buf, payload_str.size());
-                memcpy(op->buf, payload_str.c_str(), payload_str.size());
             }
         }
     }
 #endif
+    if (payload.size())
+    {
+        std::string payload_str = json11::Json(payload).dump();
+        op->req.show_conf.json_len = payload_str.size();
+        op->buf = malloc_or_die(payload_str.size());
+        op->iov.push_back(op->buf, payload_str.size());
+        memcpy(op->buf, payload_str.c_str(), payload_str.size());
+    }
     op->callback = [this, cl](osd_op_t *op)
     {
         std::string json_err;

src/client/messenger.h

@@ -75,12 +75,15 @@ struct osd_client_t
     int read_remaining = 0;
     int read_state = 0;
     osd_op_buf_list_t recv_list;
+    uint64_t read_op_id = 1;
+    bool check_sequencing = false;
 
     // Incoming operations
     std::vector<osd_op_t*> received_ops;
 
     // Outbound operations
     std::map<uint64_t, osd_op_t*> sent_ops;
+    uint64_t send_op_id = 0;
 
     // PGs dirtied by this client's primary-writes
     std::set<pool_pg_num_t> dirty_pgs;
@@ -211,7 +214,6 @@ public:
     bool has_sendmsg_zc = false;
     // osd_num_t is only for logging and asserts
     osd_num_t osd_num;
-    uint64_t next_subop_id = 1;
     std::map<int, osd_client_t*> clients;
     std::map<osd_num_t, osd_wanted_peer_t> wanted_peers;
     std::map<uint64_t, int> osd_peer_fds;

src/client/msgr_receive.cpp

@@ -223,7 +223,19 @@ bool osd_messenger_t::handle_finished_read(osd_client_t *cl)
         if (cl->read_op->req.hdr.magic == SECONDARY_OSD_REPLY_MAGIC)
             return handle_reply_hdr(cl);
         else if (cl->read_op->req.hdr.magic == SECONDARY_OSD_OP_MAGIC)
+        {
+            if (cl->check_sequencing)
+            {
+                if (cl->read_op->req.hdr.id != cl->read_op_id)
+                {
+                    fprintf(stderr, "Warning: operation sequencing is broken on client %d, stopping client\n", cl->peer_fd);
+                    stop_client(cl->peer_fd);
+                    return false;
+                }
+                cl->read_op_id++;
+            }
             handle_op_hdr(cl);
+        }
         else
         {
             fprintf(stderr, "Received garbage: magic=%jx id=%ju opcode=%jx from %d\n", cl->read_op->req.hdr.magic, cl->read_op->req.hdr.id, cl->read_op->req.hdr.opcode, cl->peer_fd);

src/client/msgr_send.cpp

@@ -14,6 +14,7 @@ void osd_messenger_t::outbox_push(osd_op_t *cur_op)
     if (cur_op->op_type == OSD_OP_OUT)
     {
         clock_gettime(CLOCK_REALTIME, &cur_op->tv_begin);
+        cur_op->req.hdr.id = ++cl->send_op_id;
     }
     else
     {

src/cmd/cli_describe.cpp

@@ -147,7 +147,6 @@ struct cli_describe_t
                 .describe = (osd_op_describe_t){
                     .header = (osd_op_header_t){
                         .magic = SECONDARY_OSD_OP_MAGIC,
-                        .id = parent->cli->next_op_id(),
                         .opcode = OSD_OP_DESCRIBE,
                     },
                     .object_state = object_state,

src/cmd/cli_fix.cpp

@@ -159,7 +159,6 @@ struct cli_fix_t
                 .describe = {
                     .header = {
                         .magic = SECONDARY_OSD_OP_MAGIC,
-                        .id = parent->cli->next_op_id(),
                         .opcode = OSD_OP_DESCRIBE,
                     },
                     .min_inode = obj.inode,
@@ -194,7 +193,6 @@ struct cli_fix_t
                                 .sec_del = {
                                     .header = {
                                         .magic = SECONDARY_OSD_OP_MAGIC,
-                                        .id = parent->cli->next_op_id(),
                                         .opcode = OSD_OP_SEC_DELETE,
                                     },
                                     .oid = {
@@ -242,7 +240,6 @@ struct cli_fix_t
                                         .rw = {
                                             .header = {
                                                 .magic = SECONDARY_OSD_OP_MAGIC,
-                                                .id = parent->cli->next_op_id(),
                                                 .opcode = OSD_OP_SCRUB,
                                             },
                                             .inode = obj.inode,

src/osd/osd_flush.cpp

@@ -209,7 +209,6 @@ bool osd_t::submit_flush_op(pool_id_t pool_id, pg_num_t pg_num, pg_flush_batch_t
             .sec_stab = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = (uint64_t)(rollback ? OSD_OP_SEC_ROLLBACK : OSD_OP_SEC_STABILIZE),
                 },
                 .len = count * sizeof(obj_ver_id),

src/osd/osd_peering.cpp

@@ -391,7 +391,6 @@ void osd_t::submit_list_subop(osd_num_t role_osd, pg_peering_state_t *ps)
             .sec_list = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = OSD_OP_SEC_LIST,
                 },
                 .list_pg = ps->pg_num,

src/osd/osd_primary_chain.cpp

@@ -266,7 +266,6 @@ int osd_t::submit_bitmap_subops(osd_op_t *cur_op, pg_t & pg)
                     .sec_read_bmp = {
                         .header = {
                             .magic = SECONDARY_OSD_OP_MAGIC,
-                            .id = msgr.next_subop_id++,
                             .opcode = OSD_OP_SEC_READ_BMP,
                         },
                         .len = sizeof(obj_ver_id)*(i+1-prev),

src/osd/osd_primary_subops.cpp

@@ -233,7 +233,6 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop,
         subop->req.sec_rw = (osd_op_sec_rw_t){
             .header = {
                 .magic = SECONDARY_OSD_OP_MAGIC,
-                .id = msgr.next_subop_id++,
                 .opcode = (uint64_t)(wr ? (cur_op->op_data->pg->scheme == POOL_SCHEME_REPLICATED ? OSD_OP_SEC_WRITE_STABLE : OSD_OP_SEC_WRITE) : OSD_OP_SEC_READ),
             },
             .oid = {
@@ -594,7 +593,6 @@ void osd_t::submit_primary_del_batch(osd_op_t *cur_op, obj_ver_osd_t *chunks_to_
             subops[i].req = (osd_any_op_t){ .sec_del = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = OSD_OP_SEC_DELETE,
                 },
                 .oid = chunk.oid,
@@ -654,7 +652,6 @@ int osd_t::submit_primary_sync_subops(osd_op_t *cur_op)
             subops[i].req = (osd_any_op_t){ .sec_sync = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = OSD_OP_SEC_SYNC,
                 },
                 .flags = cur_op->peer_fd == SELF_FD && cur_op->req.hdr.opcode != OSD_OP_SCRUB ? OSD_OP_RECOVERY_RELATED : 0,
@@ -713,7 +710,6 @@ void osd_t::submit_primary_stab_subops(osd_op_t *cur_op)
             subops[i].req = (osd_any_op_t){ .sec_stab = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = OSD_OP_SEC_STABILIZE,
                 },
                 .len = (uint64_t)(stab_osd.len * sizeof(obj_ver_id)),
@@ -807,7 +803,6 @@ void osd_t::submit_primary_rollback_subops(osd_op_t *cur_op, const uint64_t* osd
                 subop->req = (osd_any_op_t){ .sec_stab = {
                     .header = {
                         .magic = SECONDARY_OSD_OP_MAGIC,
-                        .id = msgr.next_subop_id++,
                         .opcode = OSD_OP_SEC_ROLLBACK,
                     },
                     .len = sizeof(obj_ver_id),

src/osd/osd_scrub.cpp

@@ -65,7 +65,6 @@ void osd_t::scrub_list(pool_pg_num_t pg_id, osd_num_t role_osd, object_id min_oi
             .sec_list = {
                 .header = {
                     .magic = SECONDARY_OSD_OP_MAGIC,
-                    .id = msgr.next_subop_id++,
                     .opcode = OSD_OP_SEC_LIST,
                 },
                 .list_pg = pg_num,

src/osd/osd_secondary.cpp

@@ -198,6 +198,12 @@ void osd_t::exec_show_config(osd_op_t *cur_op)
     json11::Json req_json = cur_op->req.show_conf.json_len > 0
         ? json11::Json::parse(std::string((char *)cur_op->buf), json_err)
         : json11::Json();
+    if (req_json["check_sequencing"].bool_value())
+    {
+        auto cl = msgr.clients.at(cur_op->peer_fd);
+        cl->check_sequencing = true;
+        cl->read_op_id = cur_op->req.hdr.id + 1;
+    }
     // Expose sensitive configuration values so peers can check them
     json11::Json::object wire_config = json11::Json::object {
         { "osd_num", osd_num },

src/test/mock/messenger.cpp

@@ -21,7 +21,9 @@ osd_messenger_t::~osd_messenger_t()
 
 void osd_messenger_t::outbox_push(osd_op_t *cur_op)
 {
-    clients[cur_op->peer_fd]->sent_ops[cur_op->req.hdr.id] = cur_op;
+    auto cl = clients.at(cur_op->peer_fd);
+    cur_op->req.hdr.id = ++cl->send_op_id;
+    cl->sent_ops[cur_op->req.hdr.id] = cur_op;
 }
 
 void osd_messenger_t::parse_config(const json11::Json & config)