[jira] [Commented] (FLINK-30259) Use flink Preconditions Util instead of uncertain Assert keyword to do checking

["Wencong Liu (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-30259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17700027#comment-17700027 ] 

Wencong Liu commented on FLINK-30259:
-------------------------------------

cc [~chesnay] 

> Use flink Preconditions Util instead of uncertain Assert keyword to do checking
> -------------------------------------------------------------------------------
>
>                 Key: FLINK-30259
>                 URL: https://issues.apache.org/jira/browse/FLINK-30259
>             Project: Flink
>          Issue Type: Improvement
>          Components: Deployment / Kubernetes, Formats (JSON, Avro, Parquet, ORC, SequenceFile), Table SQL / Planner
>    Affects Versions: 1.16.0
>            Reporter: Ran Tao
>            Priority: Major
>
> The code of some modules of the current Flink project uses the 'assert' keyword of java to do checking, which actually depends on the enablement of the -enableassertions (-ea) option (default is false, which means some assert code can not work), otherwise it may lead to unexpected behavior. In fact, flink already has a mature Preconditions tool, we can use it to replace 'assert' keyword. it is more clean and consistent with flink.
> The following is an example of some snippets (by using idea, we can find other places ). 
> RowDataPrintFunction
> {code:java}
> @Override
>         public void invoke(RowData value, Context context) {
>             Object data = converter.toExternal(value);
>             assert data != null;
>             writer.write(data.toString());
>         }
> {code}
> e.g. if assert not enable,data.toString() will cause NPE.
> KubernetesUtils
> {code:java}
>     public static KubernetesConfigMap checkConfigMaps(
>             List<KubernetesConfigMap> configMaps, String expectedConfigMapName) {
>         assert (configMaps.size() == 1);
>         assert (configMaps.get(0).getName().equals(expectedConfigMapName));
>         return configMaps.get(0);
>     }
> {code}
> e.g. if assert not enable,configMaps.get(0)will cause NPE.
> RocksDBOperationUtils
> {code:java}
> if (memoryConfig.isUsingFixedMemoryPerSlot()) {
>                 assert memoryConfig.getFixedMemoryPerSlot() != null;
>                 logger.info("Getting fixed-size shared cache for RocksDB.");
>                 return memoryManager.getExternalSharedMemoryResource(
>                         FIXED_SLOT_MEMORY_RESOURCE_ID,
>                         allocator,
>                         // if assert not enable,  here will cause NPE.
>                         memoryConfig.getFixedMemoryPerSlot().getBytes());
>             } else {
>                 logger.info("Getting managed memory shared cache for RocksDB.");
>                 return memoryManager.getSharedMemoryResourceForManagedMemory(
>                         MANAGED_MEMORY_RESOURCE_ID, allocator, memoryFraction);
>             }
> {code}
> e.g. if assert not enable, RocksDBOperationUtils#memoryConfig.getFixedMemoryPerSlot().getBytes())  will cause NPE.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)