[Tarantool-patches] [PATCH v3] wal: introduce limits on simultaneous writes

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Tue Mar 2 01:05:26 MSK 2021


Hi! Thanks for the fixes!

>>> @@ -159,6 +276,13 @@ journal_write(struct journal_entry *entry)
>>>   static inline int
>>>   journal_write_async(struct journal_entry *entry)
>>>   {
>>> +    /*
>>> +     * It's the job of the caller to check whether the queue is full prior
>>> +     * to submitting the request.
>>> +     */
>>> +    assert(!journal_queue_is_full() || journal_queue.is_ready);
>>> +    journal_queue_on_append(entry);
>> 8. Probably must assert that waiters list is empty. Otherwise you
>> could go out of order.
> 
> It's not empty by the time the first entry gets to 'journal_write_async'.
> Everyone else is waken up, but not yet removed from the queue.
> 
> Looks like we cannot determine whether a write is called after waiting in queue
> or not.

It bothers me a lot, the rest looks good. We don't have any protection against
a reorder, even not an assertion. How about this? (I didn't test):

====================
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -69,8 +69,11 @@ void
 journal_queue_wakeup(void)
 {
 	struct rlist *list = &journal_queue.waiters;
-	if (!rlist_empty(list) && !journal_queue_is_full())
+	if (!rlist_empty(list) && !journal_queue_is_full()) {
 		fiber_wakeup(rlist_first_entry(list, struct fiber, state));
+		--journal_queue.waiter_count;
+		assert(journal_queue.waiter_count >= 0);
+	}
 }
 
 void
@@ -84,7 +87,6 @@ journal_queue_wait(void)
 	 * Will be waken up by either queue emptying or a synchronous write.
 	 */
 	fiber_yield();
-	--journal_queue.waiter_count;
 	journal_queue_wakeup();
 }
 
@@ -96,5 +98,6 @@ journal_queue_flush(void)
 	struct rlist *list = &journal_queue.waiters;
 	while (!rlist_empty(list))
 		fiber_wakeup(rlist_first_entry(list, struct fiber, state));
+	journal_queue.waiter_count = 0;
 	journal_queue_wait();
 }
diff --git a/src/box/journal.h b/src/box/journal.h
index 5f0e0accd..ea56043e2 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -260,6 +260,7 @@ journal_write_async(struct journal_entry *entry)
 	 * to submitting the request.
 	 */
 	journal_queue_on_append(entry);
+	assert(journal_queue.waiter_count == 0);
 
 	return current_journal->write_async(current_journal, entry);
 }


More information about the Tarantool-patches mailing list