The brtloader is now at 95.03% lines covered and 84.64% of branches taken. ...

The brtloader is now at 95.03% lines covered and 84.64% of branches taken.  That's pretty good.  We can work to remove a few more, but it's diminishing returns from here.  [t:2647] Refs #2647.


git-svn-id: file:///svn/toku/tokudb@20537 c7de825b-a66e-492c-adef-691d508d4ae1

newbrt/brtloader.c

@@ -1512,7 +1512,6 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
     if (result==0 && to_q) {
 	XMALLOC(output_rowset); // freed in cleanup
 	int r = init_rowset(output_rowset, memory_per_rowset(bl));
-	lazy_assert(r==0);
         if (r!=0) result = r;
     }
     
@@ -1526,7 +1525,7 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
             r = pqueue_pop(pq, &node);
             if (r!=0) {
 		result = r;
-		printf("%s:%d returning\n", __FILE__, __LINE__); // comment this line out when we get a test that tests this code path.
+		lazy_assert(0);
 		break;
             }
             mini = node->i;
@@ -1536,10 +1535,12 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
 		BL_TRACE(blt_do_i);
 		r = queue_enq(q, (void*)output_rowset, 1, NULL);
 		BL_TRACE(blt_fractal_enq);
-		lazy_assert(r==0);
+		if (r!=0) {
+		    result = r;
+		    break;
+		}
                 XMALLOC(output_rowset); // freed in cleanup
 		r = init_rowset(output_rowset, memory_per_rowset(bl));
-		lazy_assert(r==0);
                 if (r!=0) {        
                     result = r;
                     break;
@@ -1554,7 +1555,6 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
 	} else {
             // write it to the dest file
 	    r = loader_write_row(&keys[mini], &vals[mini], dest_data, dest_stream, &dataoff[mini], bl);
-	    lazy_assert(r==0);
             if (r!=0) {
                 result = r;
                 break;
@@ -1572,7 +1572,6 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
 		    toku_free(keys[mini].data);  keys[mini].data = NULL;
 		    toku_free(vals[mini].data);  vals[mini].data = NULL;
 		} else {
-		    printf("%s:%d returning\n", __FILE__, __LINE__);
                     lazy_assert(0);
                     result = r;
                     break;
@@ -1612,7 +1611,6 @@ int toku_merge_some_files_using_dbufio (const BOOL to_q, FIDX dest_data, QUEUE q
 	BL_TRACE(blt_do_i);
 	int r = queue_enq(q, (void*)output_rowset, 1, NULL);
 	BL_TRACE(blt_fractal_enq);
-	lazy_assert(r==0); // 	
         if (r!=0) 
             result = r;
         else 
@@ -1794,6 +1792,7 @@ int merge_files (struct merge_fileset *fs,
 	if (result!=0) break;
     }
     if (result) brt_loader_set_panic(bl, result, TRUE);
+
     {
 	int r = queue_eof(output_q);
 	if (r!=0 && result==0) result = r;

newbrt/queue.c

@@ -91,9 +91,13 @@ int queue_enq (QUEUE q, void *item, u_int64_t weight, u_int64_t *total_weight_af
     }
     assert(!q->eof);
     // Go ahead and put it in, even if it's too much.
-    q->contents_weight += weight;
     struct qitem *MALLOC(qi);
-    if (qi==NULL) return errno;
+    if (qi==NULL) {
+	int r = errno;
+	toku_pthread_mutex_unlock(&q->mutex);
+	return r;
+    }
+    q->contents_weight += weight;
     qi->item = item;
     qi->weight = weight;
     qi->next   = NULL;

newbrt/tests/Makefile

@@ -110,14 +110,14 @@ check_brtloader-test-open$(BINSUF): EXTRA_ARGS=dir_$@
 check_brtloader-test-writer$(BINSUF): $(patsubst %,check_brtloader-test-writer-%, 1 2)
 	true $(SUMMARIZE_CMD)
 check_brtloader-test-writer-1: brtloader-test-writer$(BINSUF)
-	$(VGRIND) ./$< -q -r 1000000 dir.$@ $(SUMMARIZE_CMD)
+	$(VGRIND) ./$< -q -r 100000 -s dir.$@ $(SUMMARIZE_CMD)
 check_brtloader-test-writer-2: brtloader-test-writer$(BINSUF)
-	$(VGRIND) ./$< -q -r 10000000 dir.$@ $(SUMMARIZE_CMD)
+	$(VGRIND) ./$< -q -r 1000000 -s dir.$@ $(SUMMARIZE_CMD)
 
 check_brtloader-test-writer-errors$(BINSUF): $(patsubst %,check_brtloader-test-writer-errors-%, 1 2 3)
 	true $(SUMMARIZE_CMD)
 check_brtloader-test-writer-errors-1: brtloader-test-writer-errors$(BINSUF)
-	$(VGRIND) ./$< -q -w -m -u -r 1000000 dir.$@ $(SUMMARIZE_CMD)
+	$(VGRIND) ./$< -q -w -m -u -r 100000 dir.$@ $(SUMMARIZE_CMD)
 check_brtloader-test-writer-errors-2: brtloader-test-writer-errors$(BINSUF)
 	$(VGRIND) ./$< -q -s -w -m -u -r 10000 dir.$@ $(SUMMARIZE_CMD)
 check_brtloader-test-writer-errors-3: brtloader-test-writer-errors$(BINSUF)
@@ -143,6 +143,8 @@ brtloader_test$(BINSUF): ../brtloader-internal.h ../brtloader.o
 ../brtloader.$(OEXT): ../brtloader.c ../brtloader-internal.h 
 	cd $(@D) && $(MAKE) $(@F)
 
+check_brtloader-tests:  check_brtloader-test-bad-generate check_brtloader-test-extractor-errors check_brtloader-test-merge-files-dbufio check_brtloader-test-open check_brtloader-test-writer check_brtloader-test-writer-errors
+
 clean:
 	rm -rf log-test7.c.dir_*
 	rm -rf *.dir dir.*

newbrt/tests/brtloader-test-merge-files-dbufio.c

@@ -420,6 +420,7 @@ static int usage(const char *progname, int n) {
 }
 
 int test_main (int argc, const char *argv[]) {
+    int tstart = 0;
     int tend = -1;
     const char *progname=argv[0];
     argc--; argv++;
@@ -440,6 +441,9 @@ int test_main (int argc, const char *argv[]) {
 	} else if (strcmp(argv[0],"-tend") == 0) {
             argc--; argv++;
 	    tend = atoi(argv[0]);
+	} else if (strcmp(argv[0],"-tstart") == 0) {
+            argc--; argv++;
+	    tstart = atoi(argv[0]);
 	} else if (argc!=1) {
             return usage(progname, N_RECORDS);
 	}
@@ -472,10 +476,10 @@ int test_main (int argc, const char *argv[]) {
 
     {
 	int event_limit = event_count;
-	if (tend>0 && tend<event_limit) event_limit=tend;
+	if (tend>=0 && tend<event_limit) event_limit=tend;
 	if (verbose) printf("event_limit=%d\n", event_limit);
 
-    for (int i = 1; i <= event_limit; i++) {
+    for (int i = tstart+1; i <= event_limit; i++) {
         reset_event_counts();
         reset_my_malloc_counts();
         event_count_trigger = i;

src/tests/Makefile

@@ -380,11 +380,11 @@ ABORTCODE=137
 	$(MAYBEINVERTER) $(SUMMARIZE_CMD)
 
 .PHONY: %.loader
+# Generally these subtests cannot run in parallel until we fix #2645.
 %.loader: %.tdb$(BINSUF) $(PTHREAD_LOCAL)
-	echo doing ./$< fastloader && \
-	$(TDBVGRIND) ./$< $(VERBVERBOSE) && \
-	echo doing ./$< putloader && \
-	$(TDBVGRIND) ./$< -p $(VERBVERBOSE) $(MAYBEINVERTER) $(SUMMARIZE_CMD)
+	$(TDBVGRIND) ./$< $(EXTRA_ARGS) $(VERBVERBOSE)
+	$(TDBVGRIND) ./$< $(EXTRA_ARGS) -p $(VERBVERBOSE) $(MAYBEINVERTER) $(SUMMARIZE_CMD)
+loader-cleanup-test.loader: EXTRA_ARGS = -s -r 8000
 
 .PHONY: %.recover
 all.recover: $(patsubst %,test_log%.recover,$(TLRECOVER)) $(PTHREAD_LOCAL) ;
@@ -652,8 +652,8 @@ test_thread_stack.%run: test_thread_stack.%$(BINSUF) $(PTHREAD_LOCAL)
 	./$< -a -thread_stack 16384 -resume $(SUMMARIZE_CMD)
 
 
-loader-dup-test.tdbrun: $(patsubst %,loader-dup-test%.tdbrun, 0 1 2 3 4 5)
-	true $(SUMMARIZE_CMD) #State that this has passed.
+loader-dup-test.loader loader-dup-test.tdbrun: $(patsubst %,loader-dup-test%.tdbrun, 0 1 2 3 4 5)
+	true $(SUMMARIZE_CMD) #State that this has passed.  Override the default rule.
 
 loader-dup-test0.tdbrun: loader-dup-test.tdb$(BINSUF)
 	$(VGRIND) ./loader-dup-test.tdb -e 0 $(SUMMARIZE_CMD)
@@ -673,12 +673,11 @@ loader-dup-test4.tdbrun: loader-dup-test.tdb$(BINSUF)
 loader-dup-test5.tdbrun: loader-dup-test.tdb$(BINSUF)
 	$(VGRIND) ./loader-dup-test.tdb -d 1 -s -r 1000 -E -e 5 $(SUMMARIZE_CMD)
 
-loader-cleanup-test.tdbrun: $(patsubst %,loader-cleanup-test%.tdbrun, 1)
-	true $(SUMMARIZE_CMD) #State that this has passed.
+LOADER_USE_DEFAULT_TESTS = cleanup-test create-abort create-close no-puts reference-test stress-test tpch-load
 
 ## Cannot run loader-cleanup-test parallel till we fix #2645.
 #loader-cleanup-test0.tdbrun: loader-cleanup-test.tdb$(BINSUF)
 #	./loader-cleanup-test.tdb $(SUMMARIZE_CMD)
 
-loader-cleanup-test1.tdbrun: loader-cleanup-test.tdb$(BINSUF)
-	$(VGRIND) ./loader-cleanup-test.tdb -r 4000 -s $(SUMMARIZE_CMD)
+loader-tests: $(LOADER_TESTS)
+	echo $(LOADER_TESTS)

src/tests/loader-cleanup-test.c

@@ -140,6 +140,7 @@ err_type_str (enum test_type t) {
     }
     // I know that Barry prefers the single-return case, but writing the code this way means that the compiler will complain if I forget something in the enum. -Bradley
     assert(0);
+    return NULL;
 }
 
 static size_t bad_fwrite (const void *ptr, size_t size, size_t nmemb, FILE *stream) {
@@ -209,7 +210,7 @@ bad_fopen(const char *filename, const char *mode) {
     fopen_count++;
     event_count++;
     if (fopen_count_trigger == fopen_count || event_count == event_count_trigger) {
-	printf("Doing fopen_count=%d event_count=%" PRId64 "\n", fopen_count, event_count);
+	//printf("Doing fopen_count=%d event_count=%" PRId64 "\n", fopen_count, event_count);
 	errno = EINVAL;
 	rval  = NULL;
     } else {
@@ -850,7 +851,6 @@ static void run_all_tests(void) {
 	    enum test_type t = et[j];
 	    const char * write_type = err_type_str(t);
 	    int nominal = *(nomp[j]);
-	    printf("Test %s %d\n", write_type, nominal);
 	    if (verbose)
 		printf("\nNow test with induced ENOSPC/EINVAL errors returned from %s, nominal = %d\n", write_type, nominal);
 	    int i;