Conversation
bshifter
force-pushed
the
disk-buffer-fixes
branch
from
4e7cd8c to
a189d95
Compare
| writer_sck_builder); | ||
|
|
||
|
|
||
| log_writer_set_queue(self->writer, NULL); |
There was a problem hiding this comment.
I don't understand this fix. Can we discuss it?
There was a problem hiding this comment.
This is a workaround, the real resolution would be, for example, to completely get rid of log_queue_memory_usage_sub() calls in LogQueue::_unregister_shared_counters() and do the decrements locally in each implementation at the right time.
load/save operations Signed-off-by: shifter <[email protected]>
… a diskbuffer file front_cache_output get filled after front_cache limit reached Signed-off-by: shifter <[email protected]>
…g limits Signed-off-by: shifter <[email protected]>
On reload, AFsocket destinations release the previous LogQueue after creating a new one, which can wipe memory usage metrics recorded by the new LogQueue. This fix needs deeper investigation; possible side effects are unclear.This fix requires a strong examine. Signed-off-by: shifter <[email protected]>
bshifter
force-pushed
the
disk-buffer-fixes
branch
from
a189d95 to
687aa28
Compare
| else | ||
| { | ||
| return self->hdr->length + | ||
| self->hdr->backlog_pos.count + |
There was a problem hiding this comment.
backlog is missing from the new calculation, but it's not the end of the world
bazsi
left a comment
bazsi
left a comment
There was a problem hiding this comment.
Mid air collision of my comments, submitting anyway.
| } | ||
|
|
||
| if (post_op_cb && user_data) | ||
| post_op_cb(QDISK_SAVE, self->hdr, user_data); |
There was a problem hiding this comment.
I don't see why this if becoming a callback initiated from the depths of qdisk_start/stop
Rather than doing the same printouts from wherever we call qdisk_start/stop.
I don't see we are publishing information or a state that would not be available otherwise.
There was a problem hiding this comment.
we have the qdisk hdr this way. we also use it's data in debug logs. also the timing if logging is crucial.
| } | ||
|
|
||
| static gboolean | ||
| _iv_list_partition(struct iv_list_head *src, struct iv_list_head *dst, gint index) |
There was a problem hiding this comment.
This should not have an iv_list specific name, rather something that explains that it is actually splitting the list in two, starting at a specific index.
| dst->prev = src->prev; | ||
| src->prev->next = dst; | ||
| src->prev = pos->prev; | ||
| pos->prev = dst; |
There was a problem hiding this comment.
Let's not reimplement low level ivykis list functions. (Don't remember exact names but I am fairly sure there are easier to read functions for removing/adding an element)
Also applies to list iteration.
There was a problem hiding this comment.
Probably my bad, but didn't find any existing iv_function to split lists. I'm gonna double check it!
| return; | ||
| if (self->front_cache_output.len <= self->front_cache.limit) | ||
| { | ||
| iv_list_splice_tail_init(&self->front_cache_output.items, &self->front_cache.items); |
There was a problem hiding this comment.
This is not performance sensitive as much, so maybe just always using the split function would works, simplifying the function.
I am ok with keeping this a separate case.
| { | ||
| LogQueueDiskNonReliable *self = (LogQueueDiskNonReliable *) user_data; | ||
|
|
||
| if (op == QDISK_LOAD) |
There was a problem hiding this comment.
Again, the callback does not seem to be required for this one either.
There was a problem hiding this comment.
This is avoidable in case we figure out a working solution not to use callbacks here. I'm going to ask the other's opinions about it.
3 participants
This pr contains a set of diskbuffer changes: