Skip to content

Commit

Permalink
Fix: XNCI writes corrupting node data (alternative) (#2785)
Browse files Browse the repository at this point in the history 
* test: TreeSegemts also affects xncis

* fix: cleanup attribute updates; only unlink record if segment is written

* return if put dsc failed

* fix: TreeGetRecord preserve program flow and status
  • Loading branch information
@zack-vii
zack-vii committed Jun 6, 2024
1 parent 3958b92 commit ccfa885
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 56 deletions.
21 changes: 9 additions & 12 deletions treeshr/TreeGetRecord.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "treeshrp.h"
#include "treethreadstatic.h"

#define align(bytes, size) ((((bytes) + (size)-1) / (size)) * (size))
#define align(bytes, size) ((((bytes) + (size) - 1) / (size)) * (size))

static int64_t ViewDate = -1;

Expand All @@ -64,7 +64,9 @@ static inline int read_descriptor(TREE_INFO *info, int length, NCI *nci, int nid
status = TreeBADRECORD;
else if (!(nci->flags2 & NciM_NON_VMS) &&
((retsize != length) || (nodenum != nidx)))
{
status = TreeBADRECORD;
}
else
status = (MdsSerializeDscIn(data, dsc) & 1)
? TreeSUCCESS
Expand Down Expand Up @@ -130,22 +132,17 @@ int _TreeGetRecord(void *dbid, int nid_in, mdsdsc_xd_t *dsc)
status = TreeGetExtendedAttributes(
info, RfaToSeek(nci.DATA_INFO.DATA_LOCATION.rfa),
&attributes);
if (STATUS_OK &&
attributes
.facility_offset[STANDARD_RECORD_FACILITY] !=
-1)
if (STATUS_NOT_OK)
status = TreeBADRECORD;
else if (attributes.facility_offset[STANDARD_RECORD_FACILITY] != -1)
{
status = tree_get_dsc(
info, nid->tree,
attributes
.facility_offset[STANDARD_RECORD_FACILITY],
attributes
.facility_length[STANDARD_RECORD_FACILITY],
attributes.facility_offset[STANDARD_RECORD_FACILITY],
attributes.facility_length[STANDARD_RECORD_FACILITY],
dsc);
}
else if (STATUS_OK &&
attributes.facility_offset
[SEGMENTED_RECORD_FACILITY] != -1)
else if (attributes.facility_offset[SEGMENTED_RECORD_FACILITY] != -1)
{
status = _TreeGetSegmentedRecord(dbid, nid_in, dsc);
}
Expand Down
57 changes: 31 additions & 26 deletions treeshr/TreeSegments.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ static int write_property(TREE_INFO *tinfo, int64_t *offset, const char *buffer,
for (i = 0; i < FACILITIES_PER_EA; i++) \
swap_inplace(dir, int32, ptr, ext_attr->facility_length[i]); \
}
#define EXTENDED_ATTRIBUTES_SIZE 8 + FACILITIES_PER_EA *(8 + 4)
#define EXTENDED_ATTRIBUTES_SIZE 8 + FACILITIES_PER_EA * (8 + 4)
int TreePutExtendedAttributes(TREE_INFO *tinfo, EXTENDED_ATTRIBUTES *ext_attr,
int64_t *offset)
{
Expand Down Expand Up @@ -311,17 +311,10 @@ inline static void begin_extended_nci(vars_t *vars)
{
vars->attr_offset =
RfaToSeek(vars->local_nci.DATA_INFO.DATA_LOCATION.rfa);
if (vars->attr.facility_offset[STANDARD_RECORD_FACILITY] != -1)
{
vars->attr.facility_offset[STANDARD_RECORD_FACILITY] = -1;
vars->attr.facility_length[STANDARD_RECORD_FACILITY] = 0;
vars->attr_update = 1;
}
}
else
{
memset(&vars->attr, -1, sizeof(vars->attr));
vars->attr_update = 1;
vars->attr_offset = -1;
}
}
Expand Down Expand Up @@ -412,9 +405,10 @@ inline static int open_datafile_write1(vars_t *vars)
inline static void set_compress(vars_t *vars)
{
vars->compress = ((vars->local_nci.flags & NciM_COMPRESS_ON_PUT) &&
(vars->local_nci.flags & NciM_COMPRESS_SEGMENTS) &&
!(vars->local_nci.flags & NciM_DO_NOT_COMPRESS)) ?
vars->local_nci.compression_method : -1;
(vars->local_nci.flags & NciM_COMPRESS_SEGMENTS) &&
!(vars->local_nci.flags & NciM_DO_NOT_COMPRESS))
? vars->local_nci.compression_method
: -1;
}

#define NAMED_ATTRIBUTES_INDEX_SIZE \
Expand Down Expand Up @@ -594,6 +588,7 @@ static int set_xnci(vars_t *vars, mdsdsc_t *value, int is_offset)
length_t dlen = vars->local_nci.length - 8;
l_length_t ddlen = dlen + sizeof(mdsdsc_t);
status = MdsGet1Dx(&ddlen, &dsc_dtype, &dsc, 0);
RETURN_IF_STATUS_NOT_OK;
dptr = dsc.pointer;
dptr->length = dlen;
dptr->dtype = vars->local_nci.dtype;
Expand All @@ -616,11 +611,13 @@ static int set_xnci(vars_t *vars, mdsdsc_t *value, int is_offset)
break;
}
}
status =
tree_put_dsc(vars->dblist, vars->tinfo, *(int *)vars->nid_ptr, dptr,
&vars->attr.facility_offset[STANDARD_RECORD_FACILITY],
&vars->attr.facility_length[STANDARD_RECORD_FACILITY],
vars->compress);
vars->attr_update = 1;
vars->attr.facility_offset[STANDARD_RECORD_FACILITY] = -1;
RETURN_IF_NOT_OK(tree_put_dsc(
vars->dblist, vars->tinfo, *(int *)vars->nid_ptr, dptr,
&vars->attr.facility_offset[STANDARD_RECORD_FACILITY],
&vars->attr.facility_length[STANDARD_RECORD_FACILITY],
vars->compress));
vars->local_nci.flags2 &= ~NciM_DATA_IN_ATT_BLOCK;
}
else
Expand Down Expand Up @@ -650,6 +647,8 @@ static int set_xnci(vars_t *vars, mdsdsc_t *value, int is_offset)
FREE_NOW(data);
if (STATUS_OK)
{
vars->attr_update = 1;
vars->attr.facility_offset[STANDARD_RECORD_FACILITY] = -1;
status = tree_put_dsc(
vars->dblist, vars->tinfo, *(int *)vars->nid_ptr,
(mdsdsc_t *)&xd,
Expand All @@ -661,8 +660,6 @@ static int set_xnci(vars_t *vars, mdsdsc_t *value, int is_offset)
}
if (length <= 0 || STATUS_NOT_OK)
{
vars->attr.facility_offset[STANDARD_RECORD_FACILITY] = 0;
vars->attr.facility_length[STANDARD_RECORD_FACILITY] = 0;
vars->local_nci.length = 0;
vars->local_nci.DATA_INFO.DATA_LOCATION.record_length = 0;
}
Expand Down Expand Up @@ -915,14 +912,21 @@ static int save_segment_header(vars_t *vars)
}
else
{
return put_segment_header(
const int status = put_segment_header(
vars->tinfo, &vars->shead,
&vars->attr.facility_offset[SEGMENTED_RECORD_FACILITY]);
if (STATUS_OK)
{
vars->attr_update = 1;
vars->attr.facility_offset[STANDARD_RECORD_FACILITY] = -1;
vars->attr.facility_length[STANDARD_RECORD_FACILITY] = -1;
}
return status;
}
}

#define SEGMENT_INDEX_SIZE \
8 + 4 + SEGMENTS_PER_INDEX *(8 + 8 + 8 + 4 + 8 + 4 + 8 + 4 + 8 + 4)
8 + 4 + SEGMENTS_PER_INDEX * (8 + 8 + 8 + 4 + 8 + 4 + 8 + 4 + 8 + 4)
#define swap_sindex(dir, buf, sindex) \
{ \
char *ptr = buf; \
Expand Down Expand Up @@ -987,7 +991,8 @@ inline static int begin_finish(vars_t *vars)
#ifndef _WIN32
static int saved_uic = 0;
static int saved_uic32 = 0;
static void init_saved_uic() {
static void init_saved_uic()
{
if (!saved_uic)
{
saved_uic = getuid();
Expand Down Expand Up @@ -1047,7 +1052,6 @@ inline static int begin_segment_header(vars_t *vars, mdsdsc_a_t *initialValue)
memset(&vars->shead, 0, sizeof(vars->shead));
vars->shead.index_offset = -1;
vars->shead.idx = -1;
vars->attr_update = 1;
}
return TreeSUCCESS;
}
Expand Down Expand Up @@ -1735,7 +1739,8 @@ int _TreeXNciPutTimestampedSegment(void *dbid, int nid, const char *xnci,
rows_to_insert = rows_to_insert > remaining_rows_in_segment
? remaining_rows_in_segment
: rows_to_insert;
/*if STATUS_OK*/ {
if (STATUS_OK)
{
int64_t offset = vars->shead.data_offset + start_idx * bytes_per_row;
uint32_t bytes_to_insert = rows_to_insert * bytes_per_row;
if (bytes_to_insert <
Expand Down Expand Up @@ -2303,7 +2308,7 @@ int tree_put_dsc(PINO_DATABASE *dbid, TREE_INFO *tinfo, int nid_in,
unsigned char tree = nid->tree;
void *dbid_tree[2] = {(void *)dbid, (void *)&tree};
int status = MdsSerializeDscOutZ(dsc, &xd, tree_fixup_nid, dbid_tree, 0, 0,
compress,
compress,
&compressible, &ddlen, &reclen,
&dtype, &class, 0, 0, &data_in_altbuf);
if (STATUS_OK && xd.pointer && xd.pointer->class == CLASS_A &&
Expand Down Expand Up @@ -2575,12 +2580,12 @@ int TreeCopyExtended(PINO_DATABASE *dbid_in, PINO_DATABASE *dbid_out, int nid,
copy_segmented_records(tinfo_in, dbid_out, tinfo_out, nid,
&attr.facility_offset[SEGMENTED_RECORD_FACILITY],
&attr.facility_length[SEGMENTED_RECORD_FACILITY],
(compress) ? nci->compression_method : -1);
(compress) ? nci->compression_method : -1);
if (attr.facility_offset[STANDARD_RECORD_FACILITY] != -1)
copy_standard_record(tinfo_in, dbid_out, tinfo_out, nid,
&attr.facility_offset[STANDARD_RECORD_FACILITY],
&attr.facility_length[STANDARD_RECORD_FACILITY],
(compress) ? nci->compression_method : -1);
(compress) ? nci->compression_method : -1);
RETURN_IF_NOT_OK(TreePutExtendedAttributes(tinfo_out, &attr, &offset));
SeekToRfa(offset, nci->DATA_INFO.DATA_LOCATION.rfa);
int locked = 0;
Expand Down
74 changes: 56 additions & 18 deletions treeshr/testing/TreeSegmentTest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,25 @@ static int NUM_SEGS = 100;
static int SEG_SZE = 10000;
#define NODEFMTSTR "S%02d"

static int val_in = 7357;
static DESCRIPTOR_LONG(xnci_in, &val_in);

static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
static int go = 0;
static char const tree[] = "tree_test";
static int const shot = 1;
static int result = 0;
#define TEST_STATUS(m) \
({ \
int r = (m); \
if ((r & 1) == 0) \
{ \
fprintf(stdout, "%d: %d - %s\n", num, r, #m); \
pthread_mutex_lock(&mutex); \
result = 1; \
pthread_mutex_unlock(&mutex); \
} \
#define TEST_STATUS(m) \
({ \
int r = (m); \
if ((r & 1) == 0) \
{ \
fprintf(stdout, "%4d: %d - %d - %s\n", __LINE__, num, r, #m); \
pthread_mutex_lock(&mutex); \
result = 1; \
pthread_mutex_unlock(&mutex); \
} \
})

void *job(void *args)
Expand All @@ -75,6 +78,7 @@ void *job(void *args)
while (go == 0)
pthread_cond_wait(&cond, &mutex);
pthread_mutex_unlock(&mutex);
TEST_STATUS(_TreeSetXNci(DBID, nid, "BEFORE", &xnci_in));
fprintf(stderr, "%d: nid = %d\n", num, nid);
for (i = 0; i < NUM_SEGS; i++)
{
Expand All @@ -94,6 +98,7 @@ void *job(void *args)

int main(int const argc, char const *const argv[])
{
INIT_AND_FREEXD_ON_EXIT(xd);
int a = 0;
if (argc > ++a)
NUM_THREADS = atoi(argv[a]);
Expand Down Expand Up @@ -128,32 +133,32 @@ int main(int const argc, char const *const argv[])
pthread_mutex_unlock(&mutex);
for (i = 0; i < NUM_THREADS; i++)
pthread_join(threads[i], NULL);
TEST_STATUS(_TreeOpen(&DBID, tree, shot, 1));
mdsdsc_xd_t xd = {0, DTYPE_DSC, CLASS_XD, NULL, 0};
TEST_STATUS(_TreeOpen(&DBID, tree, shot, 0));
for (num = 0; num < NUM_THREADS; num++)
{
int thread_failed = 0;
sprintf(name, NODEFMTSTR, num);
TEST_STATUS(_TreeFindNode(DBID, name, &nid));
TEST_STATUS(_TreeGetRecord(DBID, nid, &xd));
if (xd.l_length == 0 || xd.pointer == NULL ||
xd.pointer->class != CLASS_R)
{
fprintf(stderr, "%d: invalid record", num);
fprintf(stderr, "%d: invalid record\n", num);
result = 1;
continue;
}
mdsdsc_r_t const *r = (mdsdsc_r_t const *)xd.pointer;
if (r->dscptrs[0]->class != CLASS_A || r->dscptrs[0]->dtype != DTYPE_W)
{
fprintf(stderr, "%d: invalid data type", num);
fprintf(stderr, "%d: invalid data type\n", num);
result = 1;
continue;
}
int16_t const *data = (int16_t const *)r->dscptrs[0]->pointer;
int arr_length = ((mdsdsc_a_t *)r->dscptrs[0])->arsize / 2;
if (arr_length != SEG_SZE * NUM_SEGS)
{
fprintf(stderr, "%d: invalid data length %d of %d", num, arr_length,
fprintf(stderr, "%d: invalid data length %d of %d\n", num, arr_length,
SEG_SZE * NUM_SEGS);
result = 1;
continue;
Expand All @@ -164,16 +169,49 @@ int main(int const argc, char const *const argv[])
{
if (data[i + j * SEG_SZE] != (int16_t)i)
{
fprintf(stderr, "%d: invalid data[i] is %d, but %d expected", num,
fprintf(stderr, "%d: invalid data[i] is %d, but %d expected\n", num,
data[i + j * SEG_SZE], i);
result = 1;
thread_failed = result = 1;
break;
}
}
if (thread_failed)
break;
}
if (thread_failed)
continue;
TEST_STATUS(_TreeSetXNci(DBID, nid, "BEFORE", &xnci_in));
TEST_STATUS(_TreeGetXNci(DBID, nid, "BEFORE", &xd));
if (xd.pointer == NULL || xnci_in.class != xd.pointer->class || xnci_in.dtype != xd.pointer->dtype)
{
fprintf(stderr, "%d: invalid xnci read\n", num);
result = 1;
continue;
}
TEST_STATUS(_TreeGetRecord(DBID, nid, &xd));
if (xd.l_length == 0 || xd.pointer == NULL ||
xd.pointer->class != CLASS_R)
{
fprintf(stderr, "%d: record broke after xnci write\n", num);
result = 1;
continue;
}
TEST_STATUS(_TreePutRecord(DBID, nid, &xnci_in, 0));
TEST_STATUS(_TreeGetXNci(DBID, nid, "BEFORE", &xd));
if (xd.pointer == NULL || xnci_in.class != xd.pointer->class || xnci_in.dtype != xd.pointer->dtype)
{
fprintf(stderr, "%d: put record broke xnci\n", num);
result = 1;
continue;
}
TEST_STATUS(_TreeGetRecord(DBID, nid, &xd));
TEST_STATUS(_TreeSetXNci(DBID, nid, "BEFORE", &xnci_in)); // update
TEST_STATUS(_TreeGetRecord(DBID, nid, &xd));
TEST_STATUS(_TreeSetXNci(DBID, nid, "AFTER", &xnci_in)); // create
TEST_STATUS(_TreeGetRecord(DBID, nid, &xd));
}
TEST_STATUS(MdsFree1Dx(&xd, NULL));
TEST_STATUS(_TreeClose(&DBID, NULL, 0));
TreeFreeDbid(DBID);
FREEXD_NOW(xd);
return result;
}

0 comments on commit ccfa885

Please sign in to comment.