[
https://issues.apache.org/jira/browse/FLINK-30259?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ran Tao updated FLINK-30259:
----------------------------
Description:
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.
Note: many calcite classes such as flink hive parser use assert many places. we
will not fix these classes. we just fix flink classes.
was:
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.
Note: many calcite classes use assert. we will not fix these classes. we just
fix flink classes.
> 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.
> Note: many calcite classes such as flink hive parser use assert many places.
> we will not fix these classes. we just fix flink classes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
