Provide memory tracking capabilities for connector splits

[arhimondr]

No description provided.

[losipiuk]

can we call openTable more times with this refactor. If so is it a problem?

[arhimondr]

KuduRecordSet is created once per split by KuduRecordSetProvider#getRecordSet. Caching the kuduTable on the KuduRecordSet should be functionally equivalent of caching it at the KuduSplit level.

[losipiuk]

Is that called just once per for split processing?

[arhimondr]

It is called once per split when AccumuloRecordSet is created

[losipiuk]

nit: drop serialized from arg names a field name. Just leave phoenixInputSplit

[arhimondr]

I named it with the serialized prefix to avoid name clash on getters (getSerializedPhoenixInputSplit and getPhoenixInputSplit, the first one is needed to mark it as jackson serializable)

[losipiuk]

It is based on implementation detail of other component. Also cache is size bound so technically it is not sure that we will get cached instance here (though very probable).
How big the type objects are? Maybe it is better to still account them here to be on the safe side.

[arhimondr]

The memory accounting primitives for Type is not very complex to implement, but also non trivial. To implement proper memory accounting the TypeSignature class and TypeSignatureParameter class will have to provide memory accounting capabilities as well. In case of TypeSignatureParameter which stores the value as Object in adds additional implementation complexity, as based on the type of parameter the value of a parameter can be of 4 different types that would all have to be accounted for.

From one perspective the complexity added is not that high. But from the practical perspective I'm not sure if it is worth it.

As I already mentioned in the comment the types are cached by TypeRegistry. Many simple types are singletons (such as BigintType). The only cache in TypeRegistry that is bounded is parametricTypeCache. But it accommodates 1000 types, and in reality it is rather unlikely to overflow (as it would mean that at the current moment the system processes queries over more than 1000 columns that all have distinct types). But even so - types are usually reused across all splits as the column handles are usually resolved only once before the split enumeration starts. Thus accounting memory for splits is more likely than not to cause double counting for the memory used by those types.

[losipiuk]

there is some more stuff in Properties (defaults, map) - not sure if set in our case.

[arhimondr]

This method should account memory for all key-value pairs stored in the Properties. However as you pointed out it may not account for all internal data structure maintained by the map (what duplicates the Properties map for efficiency) and defaults (which may shadow some of the keys). It is hard to estimate memory usage of Properties precisely, but the current estimate should be close.

Ideally the schema field should be removed completely, as it duplicates a lot of data already present in the split (prestodb/presto#13453). But that's probably a topic for a follow up.

[linzebing]

Why change from URI to String? Similarly for others parts (e.g. change to serialized page), is it for accurate memory tracking? How much of a difference does it make?

[arhimondr]

It's hard to say. As i tried to explain in the commit message it is difficult to predict how much memory does the URI use, as it has many different fields where it caches certain URI parts as well as decoded / encoded representation for some of those.