On 2020/11/09 18:02, Seino Yuki wrote:
2020-11-09 15:39 に Seino Yuki さんは書きました:
However, let me confirm the following.
Is this information really useful?
If there is no valid use case for this, I'd like to drop it.
Thought?
I thought it would be easy for users to see at a glance that if there is a case
I assumed,
if the last modified date and time is old, there is no need to adjust at all,
and if the
last modified date and time is recent, it would be easy for users to understand
that the
parameters need to be adjusted.
What do you think?
Even without the last deallocation timestamp, you can presume
when the deallocation of entries happened by periodically monitoring
the total number of deallocations and seeing those history. Or IMO
it's better to log whenever the deallocation happens as proposed upthread.
Then you can easily check the history of occurrences of deallocations
from the log file.
Regards,
+1.I think you should output the deallocation history to the log as well.
In light of the above, I've posted a patch that reflects the points made.
Regards,
Sorry. There was a typo in the documentation correction.
I'll post a patch to fix it.
Thanks for updating the patch!
+ pgss_info->dealloc = 0;
+ SpinLockInit(&pgss_info->mutex);
+ Assert(pgss_info->dealloc == 0);
Why is this assertion check necessary? It seems not necessary.
+ {
+ Assert(found == found_info);
Having pgssSharedState and pgssInfoCounters separately might make
the code a bit more complicated like the above? If this is true, what about
including pgssInfoCounters in pgssSharedState?
PGSS_FILE_HEADER needs to be changed since the patch changes
the format of pgss file?
+ /* Read pgss_info */
+ if (feof(file) == 0)
+ if (fread(pgss_info, sizeof(pgssInfoCounters), 1, file) != 1)
+ goto read_error;
Why does feof(file) need to be called here?
+pgss_info_update(void)
+{
+ {
Why is the second "{" necessary? It seems redundant.
+pgss_info_reset(void)
+{
+ {
Same as above.
+pg_stat_statements_info(PG_FUNCTION_ARGS)
+{
+ int64 d_count = 0;
+ {
Same as above.
+ SpinLockAcquire(&c->mutex);
+ d_count = Int64GetDatum(c->dealloc);
+ SpinLockRelease(&c->mutex);
Why does Int64GetDatum() need to be called here? It seems not necessary.
+ <varlistentry>
+ <term>
+ <function>pg_stat_statements_info() returns bigint</function>
+ <indexterm>
+ <primary>pg_stat_statements_info</primary>
+ </indexterm>
+ </term>
Isn't it better not to expose pg_stat_statements_info() function in the
document because pg_stat_statements_info view is enough and there
seems no use case for the function?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
