alex-plekhanov commented on a change in pull request #6554: IGNITE-11073: 
Backup page store manager, initial
URL: https://github.com/apache/ignite/pull/6554#discussion_r341223060
 
 

 ##########
 File path: 
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/filename/PdsFolderSettings.java
 ##########
 @@ -137,6 +146,13 @@ public boolean isCompatible() {
         return persistentStoreRootPath;
     }
 
+    /**
+     * @return Relative configured path of presistence data storage directory 
for the local node.
+     */
+    public String pdsNodePath() {
 
 Review comment:
   1. `pdsDir` can be absolute, in this case, `pdsNodePath` will also return an 
absolute path, not relative. In this case, you will get an error in snapshot 
creation.
   2. It looks ineffective when you change `PdsConsistentIdProcessor` and 
`PdsFolderSettings` a lot, just to expose one small method which is used in one 
class.
   3. `pdsNodePath` name doesn't match returned type `String`.
   
   I think you should better revert changes to `PdsConsistentIdProcessor` and 
`PdsFolderSettings` and create a new method inside snapshot manager class, 
which uses only `folderName()` method of this class and construct full path 
using another algorithm.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to