-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/75210/#review226952
-----------------------------------------------------------




embeddedwebserver/scripts/ranger-admin-transaction-log-migrate.sh
Lines 21 (patched)
<https://reviews.apache.org/r/75210/#comment315241>

    "PLEASE EXPORT VARIABLE $" => "Environment variable not found: ${var}"



embeddedwebserver/scripts/ranger-admin-transaction-log-migrate.sh
Lines 30 (patched)
<https://reviews.apache.org/r/75210/#comment315242>

    "Using MAX_HEAP & MIN_HEAP sizes as 1g. If need to change MAX_HEAP size 
value, then please export RANGER_ADMIN_HEAP_SIZE" => "Using heap size 
${RANGER_ADMIN_HEAP_SIZE}"



embeddedwebserver/scripts/ranger-admin-transaction-log-migrate.sh
Lines 33 (patched)
<https://reviews.apache.org/r/75210/#comment315243>

    I suggest to set min and max heap to same value:
      "-Xms1g" => "-Xms${RANGER_ADMIN_HEAP_SIZE}"



embeddedwebserver/scripts/ranger-admin-transaction-log-migrate.sh
Lines 39 (patched)
<https://reviews.apache.org/r/75210/#comment315244>

    "trxlog.out" => "trxlog_v1_migration.out"



security-admin/src/main/java/org/apache/ranger/patch/cliutil/TrxLogV2MigrationUtil.java
Lines 125 (patched)
<https://reviews.apache.org/r/75210/#comment315245>

    Consider gathering stats in createTransactionLogByTrxId() instead of 
returning true/false:
    
    if (CollectionUtils.isNotEmpty(trxLogsV2)) {
      logger.debug("transaction({}): already migrated to v2", trxId);
    
      stats.alreadyMigratedCount++;
    } else {
      ...
    
      if (!oldTrxLogList.isEmpty()) {
        ...
        logger.debug("transaction({}): migrated {} v1 records", trxId, 
oldTrxLogList.size());
    
        stats.migratedCount++;
      } else {
        logger.debug("transaction({}): no v1 records found", trxId);
    
        stats.failedCount++;
      }
    }
    
    stats.logStatsIfNeeded();
    
    public static class Stats {
      private long startTime = System.currentMillis();
      private long endTime;
      private long totalCount;
      private long migratedCount;
      private long alreadyMigratedCount;
      private long failedCount;
    
      public void logStatsIfNeeded() {
        int processedCount = getProcessedCount();
        
        if (processedCount % 1000 == 0) {
          logger.info("PROGRESS: {} of {} transactions migrated", 
processedCount, totalCount);
        }
      }
      
      private long getProcessedCount() { return migratedCount + 
alreadyMigratedCount + failedCount; }
    }



security-admin/src/main/java/org/apache/ranger/patch/cliutil/TrxLogV2MigrationUtil.java
Lines 196 (patched)
<https://reviews.apache.org/r/75210/#comment315246>

    Shouldn't userId in V1 logs be converted to corresponding loginId (string 
name)? This might require querying x_portal_user table; make sure to cache the 
values to minimize the number of calls to DB.


- Madhan Neethiraj


On Sept. 27, 2024, 1:04 p.m., Rakesh Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75210/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2024, 1:04 p.m.)
> 
> 
> Review request for ranger, Dineshkumar Yadav, Kishor Gollapalliwar, Abhay 
> Kulkarni, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, 
> sanket shelar, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-4809
>     https://issues.apache.org/jira/browse/RANGER-4809
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> With updates in RANGER-4803, Ranger will store admin audit records in 
> x_trx_log_v2 table. Prior to this, x_trx_log table was used to store these 
> records. To make earlier admin audits available to updated Ranger, existing 
> entries in x_trx_log should be migrated to x_trx_log_v2 table.
> 
> 
> Diffs
> -----
> 
>   embeddedwebserver/scripts/ranger-admin-transaction-log-migrate.sh 
> PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/patch/cliutil/TrxLogV2MigrationUtil.java
>  PRE-CREATION 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 1672c148e 
> 
> 
> Diff: https://reviews.apache.org/r/75210/diff/3/
> 
> 
> Testing
> -------
> 
> The environment variables RANGER_ADMIN_HOME, RANGER_ADMIN_CONF and 
> RANGER_ADMIN_LOG_DIR must be set correctly for the migration of transaction 
> logs to proceed.
> 
> 1)Verified that all transaction from the x_trx_log table has been migrated to 
> the x_trx_log_v2 table when the default config value is -1.
> 2)Verified that data migration from the x_trx_log table to the x_trx_log_v2 
> table follows the config 
> "ranger.admin.migrate.transaction_records.retention.days", ensuring that only 
> records within the specified number of days are migrated.
> 3)Verified that multiple records with the same trx_id from the x_trx_log 
> table are merged into a single transaction record in the x_trx_log_v2 table.
> 4)Verified that the x_trx_log_v2.change_info column is populated with the 
> combined contents of the attr_name, prev_val, and new_val columns from 
> multiple x_trx_log rows.
> 5)Verified that new transactions are being correctly stored in the 
> x_trx_log_v2 table.
> 
> 
> Thanks,
> 
> Rakesh Gupta
> 
>

Reply via email to