Skip to content

Emitting numPersistentStores instead of num stores with changelog from DiagnosticsManager#1145

Merged
rmatharu-zz merged 3 commits into
apache:masterfrom
rmatharu-zz:numPersistentStores
Aug 26, 2019
Merged

Emitting numPersistentStores instead of num stores with changelog from DiagnosticsManager#1145
rmatharu-zz merged 3 commits into
apache:masterfrom
rmatharu-zz:numPersistentStores

Conversation

@rmatharu-zz
Copy link
Copy Markdown
Contributor

No description provided.

private static final String STOP_EVENT_LIST_METRIC_NAME = "stopEvents";
private static final String CONTAINER_MB_METRIC_NAME = "containerMemoryMb";
private static final String CONTAINER_NUM_CORES_METRIC_NAME = "containerNumCores";
private static final String CONTAINER_NUM_STORES_WITH_CHANGELOG_METRIC_NAME = "numStoresWithChangelog";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can all readers of this data handle a missing entry for this? Just double checking on compatibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yup, only the asc prototype uses this currently.

return (int) getStoreNames().stream()
.map(storeName -> getStorageFactoryClassName(storeName))
.filter(factoryName -> factoryName.isPresent())
.filter(factoryName -> factoryName.get().equals(PERSISTENT_STORE_FACTORY))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is inconsistent with StoreProperties.isPersistedToDisk. Over there, it is "persisted" if it is not an in-memory store. Should we do the same here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, I was looking for exactly that.
Had to add a constant because of the module dependency.

Copy link
Copy Markdown
Contributor Author

@rmatharu-zz rmatharu-zz left a comment

Choose a reason for hiding this comment

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

addressed both the comments.

@rmatharu-zz rmatharu-zz merged commit c8f8757 into apache:master Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants