Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multislicemultiget counter metric per CF #458

Open
wants to merge 2 commits into
base: palantir-cassandra-2.2.18
Choose a base branch
from

Conversation

lyubent
Copy link
Contributor

@lyubent lyubent commented Dec 12, 2023

Tracks usage of o.a.c.thrift.CassandraServer#multiget_multislice custom thrift API.

Comment on lines 445 to 447
String keyspace = cState.getKeyspace();
Keyspace.open(keyspace).getColumnFamilyStore(column_parent.column_family).metric.multiGetMultiSliceRequestQueries.inc();
markRequestMeter(keyspace);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we confirm that this Keyspace.open() call doesn't introduce additional overhead?

Copy link
Contributor Author

@lyubent lyubent Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cost of the call is free once the KS is initialized (and initialization happens before we enable the thrift interface), and initialization happens once and only once. The initialization itself isn't cheap (runs through a sync block where the monitor only allows a single thread in). There is also a map lookup and a map put on the NonBlockingHashMap used by o.a.c.config.Schema.

source

However, we should benchmark to sanity test given that this is in the critical read path.

Copy link
Contributor Author

@lyubent lyubent Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that there is some safety that is hidden behind an assert:

public static Keyspace open(String keyspaceName)
{
    assert initialized || keyspaceName.equals(SystemKeyspace.NAME);
    return open(keyspaceName, Schema.instance, true); // calls the private Keyspace#open
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants