You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2023/12/04 06:33:37 UTC
[PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
swamirishi opened a new pull request, #5719:
URL: https://github.com/apache/ozone/pull/5719
## What changes were proposed in this pull request?
Currently native rocksdb tool are getting loaded as part of OM start irrespective of whether user's are interest to use snapshots or not.
We have seen issues with loading of this native rocks tools when users have different version of jdk. So, it would be good to provide a flag to skip loading of this native libs when users not interested to use snapshots specifically.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9799
## How was this patch tested?
Adding a flag config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1414089964
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
I was assuming that snap diff would fail if rocksDB native tool is not loaded and I asked the same in previous comment.
Looking at snapshot diff code, it doesn't seem that way.
https://github.com/apache/ozone/blob/15c77c6e9bb22a0bbf5d0080f81c9310051229a9/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java#L1123
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422373728
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
we need to check if the library is loaded, instantiation of the class should fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1413434412
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
Do we need to create new flag? Can we reuse `ozone.filesystem.snapshot.enabled`? I don't think if it would be a case when snapshot is enable and native lib is disabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422136476
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
qq: do we need to load lib again here or check? Isn't it not enough loading it as part of ManagedSSTDumpTool instantiation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1413434412
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
Do we need to create new flag? Can we reuse `ozone.filesystem.snapshot.enabled`? I don't know if it would be a case when snapshot is enable and native lib is disabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422382424
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
I get that but `PipeInputStream` is only instantiation inside `ManagedSSTDumpTool`. If instantiation of `ManagedSSTDumpTool` fails because of native lib not getting loaded, `PipeInputStream` will never be instantiated.
If `PipeInputStream` was getting instantiation independently of `ManagedSSTDumpTool`, then it makes sense to load native lib here. Another reason is that we don't check if `ozone.om.snapshot.load.native.lib` is enabled.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -4181,6 +4181,15 @@
</description>
</property>
+ <property>
+ <name>ozone.om.snapshot.native.lib.enabled.diff</name>
Review Comment:
Change this to `ozone.om.snapshot.load.native.lib` as well.
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
I get that but `PipeInputStream` is only instantiation inside `ManagedSSTDumpTool`. If instantiation of `ManagedSSTDumpTool` fails because of native lib not getting loaded, `PipeInputStream` will never be instantiated.
If `PipeInputStream` was getting instantiation independently of `ManagedSSTDumpTool`, then it makes sense to load native lib here. Another reason is that we don't check if `ozone.om.snapshot.load.native.lib` is enabled here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1414089964
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
I assumed that snap diff would fail if rocksDB native tool is not loaded and I asked the same in previous comment.
Looking at snapshot diff code, it doesn't seem that way.
https://github.com/apache/ozone/blob/15c77c6e9bb22a0bbf5d0080f81c9310051229a9/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java#L1123
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1414229317
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
I do not think that if snapshot are enabled this should be optional. I agree with @hemantk-12 that this should tied to ability to create snapshots.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5719:
URL: https://github.com/apache/ozone/pull/5719#issuecomment-1857287513
Thanks @swamirishi for the patch and @kerneltime for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1416497214
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
We do not need this native lib for creating snapshot. This is for only efficient snapdiffs. One can perform the snapshot diffs using the inefficient way. We had issues where ozone manager was crashing while loading the native lib because of platform incompatibility. In such cases one should be able to disable this flag and should be able to proceed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422382424
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
I get that but `PipeInputStream` is only initialized inside `ManagedSSTDumpTool`. If initialization of `ManagedSSTDumpTool` fails because of native lib not getting loaded, `PipeInputStream` will never be initialized. My point is native lib must have been loaded by this time. If not, then it is a problem.
If `PipeInputStream` was getting instantiation independently of `ManagedSSTDumpTool`, then it makes sense to load native lib here. Another reason is that we don't check if `ozone.om.snapshot.load.native.lib` is enabled here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5719:
URL: https://github.com/apache/ozone/pull/5719#issuecomment-1852630657
@swamirishi Please rebase your fork branch to fix the build issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422092416
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -33,6 +33,11 @@ public final class OMConfigKeys {
"ozone.om.snapshot.sst_dumptool.pool.size";
public static final int
OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT = 1;
+
+ public static final String OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF =
+ "ozone.om.snapshot.native.lib.enabled.diff";
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422382424
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
I get that but `PipeInputStream` is only initialized inside `ManagedSSTDumpTool`. If initialization of `ManagedSSTDumpTool` fails because of native lib not getting loaded, `PipeInputStream` will never be initialized.
If `PipeInputStream` was getting instantiation independently of `ManagedSSTDumpTool`, then it makes sense to load native lib here. Another reason is that we don't check if `ozone.om.snapshot.load.native.lib` is enabled here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1414097172
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -33,6 +33,11 @@ public final class OMConfigKeys {
"ozone.om.snapshot.sst_dumptool.pool.size";
public static final int
OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT = 1;
+
+ public static final String OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF =
+ "ozone.om.snapshot.native.lib.enabled.diff";
Review Comment:
nit: config sounds opposite of `ozone.om.snapshot.diff.disable.native.libs`.
May be call it `ozone.om.snapshot.load.native.lib` or something along the line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1413486763
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -289,28 +289,31 @@ public PersistentMap<String, SnapshotDiffJob> getSnapDiffJobTable() {
private Optional<ManagedSSTDumpTool> initSSTDumpTool(
final OzoneConfiguration conf) {
- try {
- int threadPoolSize = conf.getInt(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_POOL_SIZE_DEFAULT);
- int bufferSize = (int) conf.getStorageSize(
- OMConfigKeys.OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE,
- OMConfigKeys
- .OZONE_OM_SNAPSHOT_SST_DUMPTOOL_EXECUTOR_BUFFER_SIZE_DEFAULT,
- StorageUnit.BYTES);
- this.sstDumpToolExecService = Optional.of(new ThreadPoolExecutor(0,
- threadPoolSize, 60, TimeUnit.SECONDS,
- new SynchronousQueue<>(), new ThreadFactoryBuilder()
- .setNameFormat(ozoneManager.getThreadNamePrefix() +
- "snapshot-diff-manager-sst-dump-tool-TID-%d")
- .build(),
- new ThreadPoolExecutor.DiscardPolicy()));
- return Optional.of(new ManagedSSTDumpTool(sstDumpToolExecService.get(),
- bufferSize));
- } catch (NativeLibraryNotLoadedException e) {
- this.sstDumpToolExecService.ifPresent(exec ->
- closeExecutorService(exec, "SstDumpToolExecutor"));
+ if (conf.getBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_NATIVE_LIB_ENABLED_DIFF,
Review Comment:
no we should have this flag. Native lib loading can fail for certain platforms
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1422382424
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
I get that but `PipeInputStream` is only initialized inside `ManagedSSTDumpTool`. If initialization of `ManagedSSTDumpTool` fails because of native lib not getting loaded, `PipeInputStream` will never be initialized. My point is native lib must have been loaded by this time. And if it is not, then it is a problem.
If `PipeInputStream` was getting instantiation independently of `ManagedSSTDumpTool`, then it makes sense to load native lib here. Another reason is that we don't check if `ozone.om.snapshot.load.native.lib` is enabled here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 merged PR #5719:
URL: https://github.com/apache/ozone/pull/5719
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9799. Provide a flag to skip the native_rocksdb_tool loading [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5719:
URL: https://github.com/apache/ozone/pull/5719#discussion_r1425747272
##########
hadoop-hdds/rocks-native/src/main/java/org/apache/hadoop/hdds/utils/db/managed/PipeInputStream.java:
##########
@@ -43,7 +39,8 @@ public class PipeInputStream extends InputStream {
private AtomicBoolean cleanup;
PipeInputStream(int capacity) throws NativeLibraryNotLoadedException {
- if (!NativeLibraryLoader.isLibraryLoaded(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)) {
+ if (!NativeLibraryLoader.getInstance()
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org