Dear Shi,
Thanks for updating the patch! Followings are comments.
ReorderBufferCheckMemoryLimit()
```
+ /*
+ * Bail out if logical_decoding_mode is disabled and we haven't exceeded
+ * the memory limit.
+ */
```
I think 'disabled' should be '"buffered"'.
```
+ * If logical_decoding_mode is immediate, loop until there's no change.
+ * Otherwise, loop until we reach under the memory limit. One might think
+ * that just by evicting the largest (sub)transaction we will come under
+ * the memory limit based on assumption that the selected transaction is
+ * at least as large as the most recent change (which caused us to go over
+ * the memory limit). However, that is not true because a user can reduce
+ * the logical_decoding_work_mem to a smaller value before the most recent
* change.
*/
```
Do we need to pick the largest (sub)transaciton even if we are in the immediate mode?
It seems that the liner search is done in ReorderBufferLargestStreamableTopTXN()
to find the largest transaction, but in this case we can choose the arbitrary one.
reorderbuffer.h
+/* possible values for logical_decoding_mode */
+typedef enum
+{
+ LOGICAL_DECODING_MODE_BUFFERED,
+ LOGICAL_DECODING_MODE_IMMEDIATE
+} LogicalDecodingMode;
I'm not sure, but do we have to modify typedefs.list?
Moreover, I have also checked the improvements of elapsed time.
All builds were made by meson system, and the unit of each cells is second.
It seemed that results have same trends with Shi.
Debug build:
HEAD PATCH Delta
16 6.44 5.96 0.48
18 6.92 6.47 0.45
19 5.93 5.91 0.02
22 14.98 13.7 1.28
23 12.01 8.22 3.79
SUM of delta: 6.02s
Release build:
HEAD PATCH Delta
16 3.51 3.22 0.29
17 4.04 3.48 0.56
19 3.43 3.3 0.13
22 10.06 8.46 1.6
23 6.74 5.39 1.35
SUM of delta: 3.93s
I will check and report the test coverage if I can.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
