marcingminski / sqlwatch

SQL Server Performance Monitor
https://docs.sqlwatch.io
Other
428 stars 168 forks source link

Block extend events my be read past long time or not at all #407

Open jmccandrade opened 3 years ago

jmccandrade commented 3 years ago

If I understand correctly, the event file is only read when the execution_count of the sqlwatch_stage_xes_exec_count table is smaller than the execution_count of the sys.dm_xe_session_targets table. The problem is that if the event session is restarted (eg with a SQL Server restart), in the case of block events, a new event file is created automatically and this count is reset. Therefore, the event file will only be read again when the run_count from the sys.dm_xe_session_targets table reaches the run_count value from the sqlwatch_stage_xes_exec_count table, and this can take a long time. If before that hapends, SQL Server restarts again, these events will be lost.

I don't know if it would be feasible to also use the event filename in the verification, to solve (or not) this issue.

marcingminski commented 3 years ago

Thats right and good catch. This was introduced to improve performance so we do not read event file if no new events but the drawback is as you have pointed out. Good suggestion about file name check. We could also check SQL start up time and compare with the last event time, or even better, reset that table upon sql restart but this would require enabling startup procedures.

jmccandrade commented 3 years ago

One idea would be to reset the counter if the name of the file changes.

To improve the read of the files, perhaps it would be possible in the future, to use the other fields of the function fn_xe_file_target_read_file, initial_file_name, initial_offset, so that we don't neet to read always the entier file.

marcingminski commented 3 years ago

I like the idea when file changes. I tried with the offset but the offset resets when the file rolls over and it added some unnecessary complexity to the code to track the rollover and offsets. I may revisit the offset in the future though.