ADBDEV-6443: Refactor diskquota local hashmap with active tables

[RekGRpth]

Refactor diskquota local hashmap with active tables

diskquota used a local hashmap local_table_stats_map in the
gp_fetch_active_tables function. During initialization, it loaded all the
information from the table with the sizes diskquota.table_size into it using
the load_table_size function. And during normal operation, diskquota loaded
information about active tables with sizes from segments into this local
hashmap using the pull_active_list_from_seg, convert_map_to_string, and
pull_active_table_size_from_seg functions. This led to increased memory
consumption, especially during initialization, since with a large number of
active tables (and all tables were considered active during initialization),
the size of this local hashmap local_table_stats_map was quite large. This
local_table_stats_map was then used in the calculate_table_disk_usage function
to calculate the sizes of active tables, and in the dispatch_rejectmap function
to dispatch active tables to segments.

This patch completely gets rid of the local_table_stats_map local hashmap,
calculating the sizes of active tables directly when receiving results in the
load_table_size and pull_active_table_size_from_seg functions, simultaneously
filling in the active_oids text list of active tables, which is then passed to
the dispatch_rejectmap function. The new get_table_size_map_entry and
update_active_table_size functions are extracted parts of the
calculate_table_disk_usage function code related to active tables. The
invalidation logic during initialization 14b861d has been completely reverted
because it is already covered by the current changes.

---

It is easier to view the changes with the "Hide whitespace" option enabled.

[dkovalev1]

The original problem you resolving is high memory usage for hash table, right?
In this commit you replace hash map which lifetime is long with the string with serialized oids, which exists only at the initialization time, is it correct?
Also this string still have to keep all oids at once resulting in probably huge query to pass to diskquota.refresh_rejectmap UDF. So it does not resolve the problem completely, but making it easier.
Have you considered to have oids in chunks instead of keeping them in one string?
Have you considered to pass oids in binary format, it could save even more bytes and CPU cycles on parsing ?

Also, this huge array (string) is used to be passed to UDF diskquota.refresh_rejectmap to be executed on segments to update rejectmap in shared memory. This string, active_oids, loaded from the table diskquota.table_size which is distributed (or could be). Have you considered to perform local initialization on a segment to avoid passing all oids to the QD and back to segments at all?

[RekGRpth]

The original problem you resolving is high memory usage for hash table, right?

Yes

In this commit you replace hash map which lifetime is long with the string with serialized oids, which exists only at the initialization time, is it correct?

No! This line of serialized oids was there before the patch, including during initialization!

Have you considered to have oids in chunks instead of keeping them in one string?
Have you considered to pass oids in binary format, it could save even more bytes and CPU cycles on parsing ?

Unfortunately, the CdbDispatchCommand function does not support passing parameters so that the oids could be passed not as a serialized string, but as a binary array of oids, as I did earlier in the delete_from_table_size_map and update_table_size_map functions.

Also, this huge array (string) is used to be passed to UDF diskquota.refresh_rejectmap to be executed on segments to update rejectmap in shared memory. This string, active_oids, loaded from the table diskquota.table_size which is distributed (or could be). Have you considered to perform local initialization on a segment to avoid passing all oids to the QD and back to segments at all?

This is beyond the scope of this patch. The fact that the diskquota.table_size table is distributed across segments does not mean that the required sizes are on the required segment!

[dkovalev1]

In this commit you replace hash map which lifetime is long with the string with serialized oids, which exists only at the initialization time, is it correct?

No! This line of serialized oids was there before the patch, including during initialization!

Yes it was, so this patch does not resolve the problem completely.

Have you considered to have oids in chunks instead of keeping them in one string?

?

Also, this huge array (string) is used to be passed to UDF diskquota.refresh_rejectmap to be executed on segments to update rejectmap in shared memory. This string, active_oids, loaded from the table diskquota.table_size which is distributed (or could be). Have you considered to perform local initialization on a segment to avoid passing all oids to the QD and back to segments at all?

This is beyond the scope of this patch. The fact that the diskquota.table_size table is distributed across segments does not mean that the required sizes are on the required segment!

Perhaps yes, but elaborating this approach can bring even more improvement and resolve the problem completely.

[RekGRpth]

this patch does not resolve the problem completely

This patch addresses a specific issue of increased memory consumption by the local hashmap during initialization.

[RekGRpth]

elaborating this approach can bring even more improvement and resolve the problem completely.

What problem are you talking about? This patch solves only one specific problem and nothing else. There are many other problems in diskquota, but this patch does not solve them. Solving all the problems with one patch is not a good idea, such a patch will be very difficult to review.

[RekGRpth]

Have you considered to have oids in chunks instead of keeping them in one string?

?

Yes, I think I already wrote that the current implementation of direct dispatching allows dispatching only string commands without parameters.