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

[rocksdb] Enable RTTI #5130

Merged
merged 1 commit into from
Feb 7, 2019
Merged

[rocksdb] Enable RTTI #5130

merged 1 commit into from
Feb 7, 2019

Conversation

pgoodman
Copy link
Contributor

This is so that you can derive a class from things like rocksdb::AssociativeMergeOperator. Perhaps a more "correct" patch would be to see if rtti is enabled for other builds, and enable it here, though it's not clear to me if that's feasible, so I'm suggesting this.

This is so that you can derive a class from things like `rocksdb::AssociativeMergeOperator`. Perhaps a more "correct" patch would be to see if rtti is enabled for other builds, and enable it here, though it's not clear to me if that's feasible, so I'm suggesting this.
@pgoodman pgoodman changed the title Enable RTTI in RocksDB [rocksdb] Enable RTTI Jan 13, 2019
@Codiferous Codiferous self-assigned this Feb 5, 2019
@Codiferous
Copy link
Contributor

So I'm completely unfamiliar with the usage of rocksdb, so I'll rely on @pgoodman to give me some info. 😄 The documentation for rocksdb suggests that RTTI is unnecessary. Is rocksdb without RTTI limited, such as that enabling RTTI enables more features/scenarios?

If RTTI is used to enable additional features, I think we should model RTTI as an additional feature of rocksdb, rather than categorically turning it on. What're your thoughts?

@pgoodman
Copy link
Contributor Author

pgoodman commented Feb 7, 2019

@Codiferous to answer your questions:

  1. RocksDB can be used without RTTI.
  2. If one wants to configure RocksDB in any reasonable way, e.g. adding a different key comparator (used to compare/sort keys in the k/v store), then RTTI is needed. Same thing for using custom merge operators, and really any other use case that requires extending rocksdb classes.
  3. I think RTTI is required when using the CLOCK algorithm for certain block caches, which correspondingly depends on Intel TBB. But my memory on this is hazy.

I am fine with modelling it as an additional feature.

@Codiferous
Copy link
Contributor

@pgoodman We discussed this in person and came to the conclusion that your PR as-is is probably the right approach. If someone requests that we disable RTTI in the future, we can use a feature to disable RTTI. As is, I think this PR is good to go!

@Codiferous Codiferous merged commit 2f4a723 into microsoft:master Feb 7, 2019
@pgoodman pgoodman deleted the patch-1 branch February 7, 2019 23:52
azure-sdk added a commit to azure-sdk/vcpkg that referenced this pull request Jan 11, 2024
## 1.11.0 (2024-01-11)

### Features Added

- Added 'OPTIONS' HTTP method to `Azure::Core::Http::HttpMethod` enum.
- Added TLS 1.3 support to WinHTTP transport.
- Environment Log Level Listener now logs the ThreadID for the thread originating the trace.
- [[microsoft#4983]](Azure/azure-sdk-for-cpp#4983) Added support for setting `CURLOPT_CAPATH` libcurl option on Linux. (A community contribution, courtesy of _[phoebusm](https://github.com/phoebusm)_)

### Bugs Fixed

- [[microsoft#5172]](Azure/azure-sdk-for-cpp#5172) `Azure::Nullable::Emplace()` does not set `HasValue()` to `true`.
- [[microsoft#5130]](Azure/azure-sdk-for-cpp#5130) `Url::AppendPath()` and `Url::SetPath()` may end up with a double slash at the beginning of a path.
- [[microsoft#5007]](Azure/azure-sdk-for-cpp#5007) Some versions of GCC no longer include stdint.h in cstdint.

### Other Changes

- [[microsoft#4756]] (Azure/azure-sdk-for-cpp#4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations.

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- Phoebus Mak _([GitHub](https://github.com/phoebusm))_
JavierMatosD pushed a commit that referenced this pull request Jan 12, 2024
* [azure-messaging-eventhubs-cpp] Update to 1.0.0-beta.5
## 1.0.0-beta.5 (2024-01-11)

### Breaking Changes

- EventHub `ConsumerClient` and `ProcessorClient` objects now return pointers to `EventData` objects instead of `EventData` objects by value.

* [azure-core-amqp-cpp] Update to 1.0.0-beta.6
## 1.0.0-beta.6 (2024-01-11)

### Features Added

- AMQP Value reference counts are now atomic, this fixes several AMQP related crashes.

### Breaking Changes

- `MessageReceiver` returns a pointer to the received message instead of a copy.

### Bugs Fixed

- Fixed several memory leaks.
- AMQP Link Credits now work as expected.
- Integrated the fix for NVD - CVE-2024-21646.

* [azure-core-cpp] Update to 1.11.0
## 1.11.0 (2024-01-11)

### Features Added

- Added 'OPTIONS' HTTP method to `Azure::Core::Http::HttpMethod` enum.
- Added TLS 1.3 support to WinHTTP transport.
- Environment Log Level Listener now logs the ThreadID for the thread originating the trace.
- [[#4983]](Azure/azure-sdk-for-cpp#4983) Added support for setting `CURLOPT_CAPATH` libcurl option on Linux. (A community contribution, courtesy of _[phoebusm](https://github.com/phoebusm)_)

### Bugs Fixed

- [[#5172]](Azure/azure-sdk-for-cpp#5172) `Azure::Nullable::Emplace()` does not set `HasValue()` to `true`.
- [[#5130]](Azure/azure-sdk-for-cpp#5130) `Url::AppendPath()` and `Url::SetPath()` may end up with a double slash at the beginning of a path.
- [[#5007]](Azure/azure-sdk-for-cpp#5007) Some versions of GCC no longer include stdint.h in cstdint.

### Other Changes

- [[#4756]] (Azure/azure-sdk-for-cpp#4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations.

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- Phoebus Mak _([GitHub](https://github.com/phoebusm))_
Osyotr pushed a commit to Osyotr/vcpkg that referenced this pull request Jan 23, 2024
* [azure-messaging-eventhubs-cpp] Update to 1.0.0-beta.5
## 1.0.0-beta.5 (2024-01-11)

### Breaking Changes

- EventHub `ConsumerClient` and `ProcessorClient` objects now return pointers to `EventData` objects instead of `EventData` objects by value.

* [azure-core-amqp-cpp] Update to 1.0.0-beta.6
## 1.0.0-beta.6 (2024-01-11)

### Features Added

- AMQP Value reference counts are now atomic, this fixes several AMQP related crashes.

### Breaking Changes

- `MessageReceiver` returns a pointer to the received message instead of a copy.

### Bugs Fixed

- Fixed several memory leaks.
- AMQP Link Credits now work as expected.
- Integrated the fix for NVD - CVE-2024-21646.

* [azure-core-cpp] Update to 1.11.0
## 1.11.0 (2024-01-11)

### Features Added

- Added 'OPTIONS' HTTP method to `Azure::Core::Http::HttpMethod` enum.
- Added TLS 1.3 support to WinHTTP transport.
- Environment Log Level Listener now logs the ThreadID for the thread originating the trace.
- [[microsoft#4983]](Azure/azure-sdk-for-cpp#4983) Added support for setting `CURLOPT_CAPATH` libcurl option on Linux. (A community contribution, courtesy of _[phoebusm](https://github.com/phoebusm)_)

### Bugs Fixed

- [[microsoft#5172]](Azure/azure-sdk-for-cpp#5172) `Azure::Nullable::Emplace()` does not set `HasValue()` to `true`.
- [[microsoft#5130]](Azure/azure-sdk-for-cpp#5130) `Url::AppendPath()` and `Url::SetPath()` may end up with a double slash at the beginning of a path.
- [[microsoft#5007]](Azure/azure-sdk-for-cpp#5007) Some versions of GCC no longer include stdint.h in cstdint.

### Other Changes

- [[microsoft#4756]] (Azure/azure-sdk-for-cpp#4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations.

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- Phoebus Mak _([GitHub](https://github.com/phoebusm))_
TomKatom pushed a commit to TomKatom/vcpkg that referenced this pull request Feb 23, 2024
* [azure-messaging-eventhubs-cpp] Update to 1.0.0-beta.5
## 1.0.0-beta.5 (2024-01-11)

### Breaking Changes

- EventHub `ConsumerClient` and `ProcessorClient` objects now return pointers to `EventData` objects instead of `EventData` objects by value.

* [azure-core-amqp-cpp] Update to 1.0.0-beta.6
## 1.0.0-beta.6 (2024-01-11)

### Features Added

- AMQP Value reference counts are now atomic, this fixes several AMQP related crashes.

### Breaking Changes

- `MessageReceiver` returns a pointer to the received message instead of a copy.

### Bugs Fixed

- Fixed several memory leaks.
- AMQP Link Credits now work as expected.
- Integrated the fix for NVD - CVE-2024-21646.

* [azure-core-cpp] Update to 1.11.0
## 1.11.0 (2024-01-11)

### Features Added

- Added 'OPTIONS' HTTP method to `Azure::Core::Http::HttpMethod` enum.
- Added TLS 1.3 support to WinHTTP transport.
- Environment Log Level Listener now logs the ThreadID for the thread originating the trace.
- [[microsoft#4983]](Azure/azure-sdk-for-cpp#4983) Added support for setting `CURLOPT_CAPATH` libcurl option on Linux. (A community contribution, courtesy of _[phoebusm](https://github.com/phoebusm)_)

### Bugs Fixed

- [[microsoft#5172]](Azure/azure-sdk-for-cpp#5172) `Azure::Nullable::Emplace()` does not set `HasValue()` to `true`.
- [[microsoft#5130]](Azure/azure-sdk-for-cpp#5130) `Url::AppendPath()` and `Url::SetPath()` may end up with a double slash at the beginning of a path.
- [[microsoft#5007]](Azure/azure-sdk-for-cpp#5007) Some versions of GCC no longer include stdint.h in cstdint.

### Other Changes

- [[microsoft#4756]] (Azure/azure-sdk-for-cpp#4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations.

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- Phoebus Mak _([GitHub](https://github.com/phoebusm))_
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.

2 participants