FirebirdSQL / firebird

Firebird server, client and tools
https://firebirdsql.org
1.26k stars 217 forks source link

Collect network statistics and make it available for the user applications. #8310

Closed hvlad closed 1 week ago

hvlad commented 2 weeks ago

The feature makes Remote provider to collect various network statistics counters and let user applications to query it using IAttachment::getInfo() (isc_database_info()) API.

There are nine counters defined:

    fb_info_wire_out_packets = 150,
    fb_info_wire_in_packets = 151,
    fb_info_wire_out_bytes = 152,
    fb_info_wire_in_bytes = 153,
    fb_info_wire_snd_packets = 154,
    fb_info_wire_rcv_packets = 155,
    fb_info_wire_snd_bytes = 156,
    fb_info_wire_rcv_bytes = 157,
    fb_info_wire_roundtrips = 158,

Counters with "in" and "out" in its names forms "logical IO" group and describes number of packets and bytes passed from the Firebird protocol point of view.

Counters with "snd" and "rcv" in its names forms "physical IO" group and describes number of packets and bytes passed over the physical connection. Also, there is fb_info_wire_roundtrips counter in "physical IO" group that shows how many times IO direction was changed from 'SEND' to 'RECEIVE'.

If wire compression is active, values of physical bytes counters should be less than values of corresponding logical counters. If wire compression is not active, both physical bytes counters and logical bytes counters should be equal.

While counters accumulated on both sides of the connection, only client side is available for client application:

aafemt commented 2 weeks ago
  • For simplicity, it is not allowed to mix locally known info items (i.e. enlisted above) with other info items in the single call of IAttachment::getInfo().

Why? MERGE_database_info() already handle a bunch of items. Adding some more shouldn't be hard.

aafemt commented 2 weeks ago

This is an improvement. Shouldn't it to be applied to master branch only/first?

hvlad commented 2 weeks ago
  • For simplicity, it is not allowed to mix locally known info items (i.e. enlisted above) with other info items in the single call of IAttachment::getInfo().

Why? MERGE_database_info() already handle a bunch of items. Adding some more shouldn't be hard.

To not pass wire stats items into the next layer that anyway have no idea about it.

hvlad commented 2 weeks ago

This is an improvement. Shouldn't it to be applied to master branch only/first?

Usually - yes. With this case it is completely new feature and should not affect any existing functionality. And I see no reason why it should not be in v5 also.

But, formally, you are correct, I agree.

dyemanov commented 2 weeks ago

I'd expect these stats (its server-side counterpart) to be also available in some new MON$ table. Not in v5 for sure.

hvlad commented 2 weeks ago

I'd expect these stats (its server-side counterpart) to be also available in some new MON$ table. Not in v5 for sure.

Agree

AlexPeshkoff commented 2 weeks ago
  • For simplicity, it is not allowed to mix locally known info items (i.e. enlisted above) with other info items in the single call of IAttachment::getInfo().

Why? MERGE_database_info() already handle a bunch of items. Adding some more shouldn't be hard.

To not pass wire stats items into the next layer that anyway have no idea about it.

This appears to be suspicious reason. It's pretty easy to remove remote provider specific items before sending info request to server. And as it was aslready mentioned merging two sets of info items is also simple task. So where is simplicity with suggested approach?

hvlad commented 2 weeks ago
  • For simplicity, it is not allowed to mix locally known info items (i.e. enlisted above) with other info items in the single call of IAttachment::getInfo().

Why? MERGE_database_info() already handle a bunch of items. Adding some more shouldn't be hard.

To not pass wire stats items into the next layer that anyway have no idea about it.

This appears to be suspicious reason. It's pretty easy to remove remote provider specific items before sending info request to server. And as it was aslready mentioned merging two sets of info items is also simple task. So where is simplicity with suggested approach?

MERGE_database_info() is called after sending of op_info_database to the remote server. Thus is makes no help if we want and able to avoid network exchange.

As already said, this requirement was made to keep code simple. If you insist, I can add more complexity here. And - for nothing - as it is far not expected to mix wire stats items with something else in same request. It can't affect existing code as there was no wire stats items before and there is no reason to add them into existing requests.

aafemt commented 2 weeks ago

One loop through the request. For every local item fill the answer buffer and remove the item from the request. Then if request is not empty and answer buffer has some space - send request and append received answer to the answer buffer. I see no complexity here.

hvlad commented 2 weeks ago

One loop ...

Do you really think I don't know how to do it ? BTW, what role plays MERGE_database_info() in your new suggestion ?

I see no complexity here.

Good for you.

hvlad commented 2 weeks ago

Requirement to not mix local and remote handled items in the same info request is removed. Description changed accordingly.

sim1984 commented 1 week ago

HELP SET command does not display help for SET WIRE_STAT

SQL> help set;
Set commands:
    SET                    -- display current SET options
    SET AUTOddl            -- toggle autocommit of DDL statements
    SET BAIL               -- toggle bailing out on errors in non-interactive mode
    SET BLOB [ALL|<n>]     -- display BLOBS of subtype <n> or ALL
    SET BLOB               -- turn off BLOB display
    SET COUNT              -- toggle count of selected rows on/off
    SET MAXROWS [<n>]      -- limit select stmt to <n> rows, zero is no limit
    SET ECHO               -- toggle command echo on/off
    SET EXPLAIN            -- toggle display of query access plan in the explained form
    SET HEADING            -- toggle display of query column titles
    SET KEEP_TRAN_params   -- toggle to keep or not to keep text of following successful SET TRANSACTION statement
    SET LIST               -- toggle column or table display format
    SET NAMES <csname>     -- set name of runtime character set
    SET PER_TABle_stats    -- toggle display of detailed per-table statistics
    SET PLAN               -- toggle display of query access plan
    SET PLANONLY           -- toggle display of query plan without executing
    SET SQL DIALECT <n>    -- set sql dialect to <n>
    SET STATs              -- toggle display of performance statistics
    SET TIME               -- toggle display of timestamp with DATE values
    SET TERM <string>      -- change statement terminator string
    SET WIDTH <col> [<n>]  -- set/unset print width to <n> for column <col>

All commands may be abbreviated to letters in CAPitals
hvlad commented 1 week ago

HELP SET command does not display help for SET WIRE_STAT

WIll add it, thanks

hvlad commented 1 week ago

HELP SET command does not display help for SET WIRE_STAT

WIll add it, thanks

Done

pavel-zotov commented 4 days ago

::: QA note ::: Test currently verifies only ability to obtain in ISQL wire counters and statistics as described in the doc. More complex test(s) will be implemented after firebird-driver become to recognize appropriate API changes.